c4-bot-9 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L154

Vulnerability details

Impact

The identified vulnerability allows a malicious borrower to engage in a Denial-of-Service (DOS) attack by disrupting (reverting) the liquidation process for an undercollateralized borrowed position. This manipulation maximizes the likelihood of the targeted position becoming insolvent, potentially leading to financial losses and compromising the integrity of the lending protocol.

The DoS will be at least 15 minutes and can be much longer.

Proof of Concept

Liquidation of a position can be initiated if the ratio of collateral / borrowed USDS falls below the minimum value (default 110%). Any user can initiate it by calling CollateralAndLiquidity.liquidateUser().

function liquidateUser(address wallet) external nonReentrant {
  .
  .
  .
  // Decrease the user's share of collateral as it has been liquidated and they no longer have it.
  _decreaseUserShare(wallet, collateralPoolID, userCollateralAmount, true);
  .
  .
  .
}

This function calls an internal function _decreaseUserShare(), as shown above, to decrease the user shares of collateral. This function has multiple parameters, one of which is bool useCooldown. If useCooldown=true then the wallet's cooldown period will be checked during liquidation.

The duration of cooldown period can range from 15 minutes to 6 hours which is set by the DAO, as described in StakingConfig.

The cooldown period is unique for each wallet and is set to block.timestamp + stakingConfig.modificationCooldown() whenever a user's shares are increased or decreased.

So if block.timestamp < user.cooldownExpiration, the transaction will revert because the cooldown period of the wallet to be liquidated haven't expired.

The holder of a position can always extend the cooldownExpiration time by adding a relatively small amount of collateral.

Alice have borrowed 10,000 USDS and has a collateral / borrowed ratio < 110%
1. This position can be liquidated
Alice deposits some collateral worth 1 USDS
1. Now cooldownExpiration = block.timestamp + 1 hour
A user calls liquidateUser() to liquidate this position
1. Since block.timestamp < cooldownExpiration, the position cannot be liquidated for 1 hour
Alice can keep on depositing a small amount or frontrun the liquidate() transaction to avoid liquidation
During market turbulence, the collateral / borrowed ratio can fall below 100% and much lower causing the position to become insolvent.

function _decreaseUserShare(
address wallet,
bytes32 poolID,
uint256 decreaseShareAmount,
bool useCooldown
) {
  .
  .
  .
    if (useCooldown)
      if (msg.sender != address(exchangeConfig.dao())) // DAO doesn't use the cooldown
      {
        require(block.timestamp >= user.cooldownExpiration, 'Must wait for the cooldown to expire');

        // Update the cooldown expiration for future transactions
        user.cooldownExpiration = block.timestamp + stakingConfig.modificationCooldown();
      }
  .
  .
  .
}

Tools Used

Manual review

Recommended Mitigation Steps

In CollateralAndLiquidity.liquidateUser(), pass false to useCooldown parameter

Before:

  _decreaseUserShare(wallet, collateralPoolID, userCollateralAmount, true);

After:

  _decreaseUserShare(wallet, collateralPoolID, userCollateralAmount, false);

Assessed type

DoS

c4-judge commented 5 months ago

Picodes marked the issue as duplicate of #891