Summary

In certain scenarios, when the prices of WETH or WBTC experience a significant drop, undercollateralized positions become impossible to liquidate due to an arithmetic underflow. This issue poses a denial-of-service risk to the liquidate function, rendering it ineffective in such situations. As a result, undercollateralized positions remain unaddressed, putting the protocol at financial risk.

Vulnerability Details

Let's assume the following scenario

User 1

• Deposit 1 ETH, ETH price is $2000, his total collateral is $2000, and he mints $1000 DSC.

• ETH price drops to $500, the user's health factory is below minimum, and he can (and should) be liquidated.

User 2

• Deposit 4 ETH, ETH price is 500$, his total collateral is $2000, and he mints $1000 DSC.

• User 2 tries to Liquidate User 1 by covering his $1000 DSC debt, he calls: liquidate(weth, user1, 1000).

• The getTokenAmountFromUsd(weth, 1000) will be triggered and since the ETH price is $500, it will return 2 ETH to be paid from User 1 to User 2 .

• The _redeemCollateral(user1, user2, weth, 2.2) (2 ETH +10% Bonus) will be triggered and then the following line of code will revert the transaction due to an underflow: s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral; (1 - 2 = -1)

Impact

Undercollateralized positions remain unaddressed, putting the protocol at financial risk.

Tools Used

VSCode

Recommendations

Our goal is to enable liquidations for positions that are significantly undercollateralized. To address this issue, I propose two potential solutions:

Solution 1

In the liquidate function, we can address this issue by verifying whether the result of getTokenAmountFromUsd(collateral, debtToCover) is greater than the liquidated user's debt (s_collateralDeposited[from][tokenCollateralAddress]).

If this condition holds true, we proceed with redeeming all the deposited collateral and burning the corresponding amount of DSC tokens proportional to the redeemed collateral.

This approach adds additional calculations to determine the precise amount of DSC tokens to burn. Additionally, it introduces an approval risk for the user who called the liquidate function, as they approve the contract to spend more DSC tokens than those that are actually burned.

Solution 2 (Less effective)

In the liquidate function verify whether the result of getTokenAmountFromUsd(collateral, debtToCover) is greater than the liquidated user's debt (s_collateralDeposited[from][tokenCollateralAddress]).

In case the user is trying to liquidate "too much collateral" revert the transaction with a custom error, so the user can call the liquidate function again with a lower number.