A user can grief the protocol to lose funds, by preventing his position from getting liquidated and ultimately making his collateral insolvent

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/StakingRewards.sol#L67

Vulnerability details

Impact

The main impact of this vulnerability is that liquidators will not be able to liquidate the positions. And as time passes on, this bad debt will ultimately make the collaterals insolvent. This will affect the protocol, as it will lose funds.

In CollateralAndLiquidity contract, when a user deposits collateral through depositCollateralAndIncreaseShare , he can borrow USDS. Let's assume that the user has borrowed X amt of USDS which is maximum amount he can borrow against his deposited collateral.
One thing to note here is that when depositCollateralAndIncreaseShare is called, it will trigger a cooldown period (currently 1 hour). This is a security mechanism to prevent reward hunting. Now the user can't increase or decrease his shares for 1 hour.

Now say the market has crashed, and the price of WETH and WBTC has gone down. Due to this, the user's position has also become liquidateble. So, a liquidator calls liquidateUser to liquidate the user's position. This will remove the liquidity and decrease the user's share.

But the user can prevent his position from getting liquidated, by frontrunning the liquidateUser function call and depositing 101 wei of WETH and WBTC using the depositCollateralAndIncreaseShare function first. Now as the depositCollateralAndIncreaseShare function will get executed first because of front-running, so this will trigger the cooldown period of 1 hour. Now after this transaction, the liquidateUser function will get executed, but it will revert because of the cooldown period. That is because, the liquidateUser function calls _decreaseUserShare function which will check if block.timestamp >= user.cooldownExpiration , and as this will be false, so the transaction will revert.

Now the user can continue preventing his position from liquidation, by front-running liquidators, or he can submit 101 wei after every stakingConfig.modificationCooldown() time, so that the cooldown period never expires.

In the meantime, the price of WETH and WBTC may go further down, which may make the position insolvent. This means that the protocol will surely lose funds.

Proof of Concept

A test case is presented below, representing how the functions would be executed when front-run. It is a representation , i.e I have not showcased how to do the front-run by analysing mempool , and have simply shown the order of execution of the functions when front-runned.

function testcounter() external {
  // charlie is a random user depositing collateral
  vm.startPrank( charlie );
  collateralAndLiquidity.depositCollateralAndIncreaseShare(wbtc.balanceOf(charlie), weth.balanceOf(charlie), 0, block.timestamp, false );
  vm.stopPrank();

  // alice is the attacker account
  vm.startPrank( alice );		 
  collateralAndLiquidity.depositCollateralAndIncreaseShare(wbtc.balanceOf(alice)/3, weth.balanceOf(alice)/3, 0, block.timestamp, false );
  uint256 maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
  collateralAndLiquidity.borrowUSDS( maxUSDS);
  vm.stopPrank();

  // Market crashes
  _crashCollateralPrice();
  vm.warp( block.timestamp + 1 days );

  // Alice frontruns and deposits 101 wei before the liquidator tries to liquidate his position
   vm.startPrank(alice);
   bool x = collateralAndLiquidity.canUserBeLiquidated(alice);
   console.log(x);
   collateralAndLiquidity.depositCollateralAndIncreaseShare(101, 101, 0, block.timestamp, false );
   vm.stopPrank();
     
   // this will revert because of the cooldown revert   
   vm.startPrank(charlie);
   collateralAndLiquidity.liquidateUser(alice);
   vm.stopPrank();
 }

Tools Used

Manula review

Recommended Mitigation Steps

Cooldown period should not affect liquidation process. Make exception.

Assessed type

Timing

[c4-judge]

Picodes marked the issue as duplicate of #312

[c4-judge]

Picodes marked the issue as satisfactory