WHILE WITHDRAWING COLLATERAL THE USER WILL NOT ALWAYS KEEP THE INIITAL COLLATERAL RATIO DUE TO ROUNDING DOWN HAPPENING IN THE `requiredCollateralValueAfterWithdrawal` VALUE CALCULATION

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L249-L250 https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L257-L261

Vulnerability details

Impact

The CollateralAndLiquidity.maxWithdrawableCollateral function is used to calculate the maximum amount of collateral that can be withdrawn while keeping the collateral ratio above a default of 200%. This collateral ratio known as initialCollateralRatioPercent is set to 200 by default but can be in the range of 150 to 300 with an adjustment of 25.

When withdrawing the collateral by a user it is required that the user keeps at least the inital collateral ratio. This calculation is performed as shown below:

    uint256 requiredCollateralValueAfterWithdrawal = ( usdsBorrowedByUsers[wallet] * stableConfig.initialCollateralRatioPercent() ) / 100;

But the issue here is that the requiredCollateralValueAfterWithdrawal value is rounded down and not rounded up. Hence as a result user does not keep at least the initial collateral ratio and the collateral ratio will be slightly less than the initialCollateralRatioPercent.

Now let's consider the following scenario:

1. User A decides to withdraw collateral amount from the salty collateral pool.
2. Let's assume His currently borrowed amount is USDS 2240 and the initialCollateralRatioPercent is 175 (can be between 150 to 300 with an adjustment of 25).
3. Hence the requiredCollateralValueAfterWithdrawal = (2245 * 175) / 100 = 392875 / 100 = 3928.75.
4. But since the requiredCollateralValueAfterWithdrawal is rounded down there will be a truncation and the 3928 will be stored as the requiredCollateralValueAfterWithdrawal.
5. But when we again calculate the collateral ratio for the requiredCollateralValueAfterWithdrawal it is as follows : (3928 / 2245) * 100 = 174.966592428.
6. Hence it is clear that the 174.966592428 < 175 and this means that at least the initial collateral ratio is not kept in place during the withdrawal.

7. Furthermore the maxWithdrawableValue is calculated as shown below:

   uint256 maxWithdrawableValue = userCollateralValue - requiredCollateralValueAfterWithdrawal;

As a result the user is able to withdraw maxWithdrawableValue more than he is eligible to withdraw had the initial collateral ratio was kept in place. Here since the requiredCollateralValueAfterWithdrawal value is less due to rounding down the maxWithdrawableValue amount is more after the substraction.

As a result this breaks the requirement that when withdrawing the collateral by a user it is required that the user keeps at least the initial collateral ratio. As it was proved above due to rounding down happening during the calculation of the requiredCollateralValueAfterWithdrawal the initial collateral ratio is not complemented and the resultant collateral ratio after withdrawal is less than the initialCollateralRatioPercent.

Proof of Concept

        // When withdrawing, require that the user keep at least the inital collateral ratio (default 200%)
        uint256 requiredCollateralValueAfterWithdrawal = ( usdsBorrowedByUsers[wallet] * stableConfig.initialCollateralRatioPercent() ) / 100;

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L249-L250

        // The maximum withdrawable value in USD
        uint256 maxWithdrawableValue = userCollateralValue - requiredCollateralValueAfterWithdrawal;

        // Return the collateralAmount that can be withdrawn
        return userCollateralAmount * maxWithdrawableValue / userCollateralValue;

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L257-L261

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to round up the requiredCollateralValueAfterWithdrawal value after the calculation, such that the initialCollateralRatioPercent will always be complemented and will not be less than that value at least slightly. As a result when withdrawing the collateral by a user, the user will always keep at least the initial collateral ratio.

Assessed type

Other

[Picodes]

The impact is minimal as there should be some margin on the collateral ratio requirements and collateral should be at least dust.

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)