c4-bot-5 commented 5 months ago

Lines of code

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

Vulnerability details

Impact

The CollateralAndLiquidity.findLiquidatableUsers function is used to find the list of borrowers who does not complement atleast the minimumCollateralRatioPercent and are eligible to be liquidated.

To evaluate whether a a single borrowed wallet is liquidatable based on its borrowed amount and minimumCollateralRatioPercent is perfomed as shown below:

            {
            address wallet = _walletsWithBorrowedUSDS.at(i); //@audit-info - get the borrowed address

            // Determine the minCollateralValue a user needs to have based on their borrowedUSDS
            uint256 minCollateralValue = (usdsBorrowedByUsers[wallet] * stableConfig.minimumCollateralRatioPercent()) / 100; //@audit-issue - this should round up and since it rounds down the user can have less than the minimum collateral value required
            //@audit-info - calculate the minimum collateral value a user should have based on teh borrowedUSDS
            // Determine minCollateral in terms of minCollateralValue
            uint256 minCollateral = (minCollateralValue * totalCollateralShares) / totalCollateralValue; //@audit-issue - this should round up and since it rounds down the user can have less than the minimum collateral liquidity required
            //@audit-info - find the minimum collateral liquidity amount a user should have
            // Make sure the user has at least minCollateral
            if ( userShareForPool( wallet, collateralPoolID ) < minCollateral ) //@audit-issue - since above values round down a user who is liquidatable is not considered liquidatable.
                liquidatableUsers[count++] = wallet; //@audit-info - add to the liquidatable users array since he does not have enough collateral liqudiity
            } //@audit-issue - if not for round down minCollateral could have been 152 but with rounding down it is 150. Hence if the `userShareForPool( wallet, collateralPoolID ) == 150` then it is considered as not liquidable

As it is evident from the above code snippet the minCollateralValue is rounded down. As a result the minCollateralValue does not complement the minimumCollateralRatioPercent. Due to the rounding down of the minCollateralValue the actual collateral ratio now will be slightly less than the minimumCollateralRatioPercent which is not the expected funcitonality or behaviour of the salty protocol when it comes to liquidations. Here the rounding is happening in favour of the borrower and not in favour of the protocol

Furthermore minCollateral value calculation is also rounded down. since it rounds down the user can have less than the minimum collateral liquidity required. Since the minCollateral value is calculated using the minCollateralValue which was rounded down and the minCollateral value is itself rounded down the resulting minCollateral value does not complement the minimumCollateralRatioPercent value. Which means the calculated minCollateral value for the user will result in a lower collateral ratio than the minimumCollateralRatioPercent.

The following check is performed to verify whether the wallet address is liquidatable.

            if ( userShareForPool( wallet, collateralPoolID ) < minCollateral ) //@audit-issue - since above values round down a user who is liquidatable is not considered liquidatable.
                liquidatableUsers[count++] = wallet; //@audit-info - add to the liquidatable users array since he does not have enough collateral liqudiity
            }

Here the user liquidity share of the collateral pool is checked against the calculated minCollateral value. If the user liquidity share of the collateral pool is less than the minCollateral value the wallet is considered liquidatable. But the issue here is that the minCollateral value is a value which was rounded down twice and is a value less than what it should be.

For example assume the calculated minCollateral value is 2500. And the userShareForPool value is 2501. As a result the wallet is not liquidatable since the userShareForPool( wallet, collateralPoolID ) > minCollateral. But due to rounding down minCollateral value does not complement the minimumCollateralRatioPercent ratio. If the minimumCollateralRatioPercent ratio was complemented then the minCollateral value should be more than the current value.

Had the minCollateral value was rounded up during its calculation rather than rounding down then the minCollateral value could be 2502. In which case the userShareForPool value of 2501 will be less than the 2502. In this case the wallet address is considered liquidatable.

Hence it is clear from the above explanation the rounding down of the minCollateral value in the CollateralAndLiquidity.findLiquidatableUsers function could lead to erroneous results with regards to the liquidatable wallets. The wallets which should be considered liquidatable are not considered so due to the rounding down of the minCollateral value.

Proof of Concept

                {
                address wallet = _walletsWithBorrowedUSDS.at(i);

                // Determine the minCollateralValue a user needs to have based on their borrowedUSDS
                uint256 minCollateralValue = (usdsBorrowedByUsers[wallet] * stableConfig.minimumCollateralRatioPercent()) / 100;

                // Determine minCollateral in terms of minCollateralValue
                uint256 minCollateral = (minCollateralValue * totalCollateralShares) / totalCollateralValue;

                // Make sure the user has at least minCollateral
                if ( userShareForPool( wallet, collateralPoolID ) < minCollateral )
                    liquidatableUsers[count++] = wallet;
                }

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

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to perform the rounding in favor of the protocol. Hence it is recommended to round up the minCollateralValue value calculation and the minCollateral value calculation such that those values complement the minimumCollateralRatioPercent ratio. Furthermore this will correctly choose the liquidatable wallets out of the borrowed wallets which do not at least has the minimum collateral ratio.