high

Liquidation Is Prevented Due To Strict Implementation of Liqudation Bonus

Reward

Total

1037.79 USDC

63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
63.28 USDC
Selected
88.59 USDC
63.28 USDC
63.28 USDC
Selected Submission

Liquidation Is Prevented Due To Strict Implementation of Liqudation Bonus

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L229-L262

Summary

The issue arises due to the strict implementation of the liquidation bonus, which prevents liquidation when a user is between 100% to 110% over-collateralized. When a user's health factor falls below a certain threshold, liquidation should occur; however, the strict bonus implementation results in insufficient funds for liquidation, leading to the transaction being reverted.

The vulnerability allows users to avoid complete liquidation, even when their health factor drops to the critical range, which is problematic for the protocol's stability and security. The issue is more likely to occur when multiple types of collateral are used, and the value of one collateral crashes.

To demonstrate the vulnerability's impact, a proof of concept and a test case have been executed, highlighting the scenario where a liquidator is unable to liquidate a user's debt completely due to insufficient collateral, leading to transaction reversion.

I recommend a mitigation step to modify the liquidation bonus calculation when the health factor is between 100% to 110%. By adjusting the liquidation bonus to the maximum positive not-zero possible amount rather than a fixed value of 1.1 * liquidationAmount, the vulnerability can be addressed.

Vulnerability Details

There is a valuable threshold when a user is 100% to 110% over-collateralized, the user must get liquidated. However, because of the strict implementation of the liquidation bonus, when the user reaches under 110% over-collateral, they cannot get liquidated fully anymore.

The reason is that when a user gets liquidated, 10% of the amount of liquidation will be sent to the liquidator as the liquidation bonus; however, if the user is not able to provide the liquidation bonus completely in their account, the liquidation will be reverted, because the user does not have sufficient funds.

Impact

When the health factor is between 100% and 110%, the liquidator cannot pay the debt partially, because the health factor is not going to be improved. Also, the liquidator cannot pay the debt completely, because the borrower does not have enough funds to pay the liquidation bonus. So, the borrower is never going to get liquidated.

Consider Alice has x as the collateral, `y` as the debt and the liquidation bonus is 10%. Consider Bob wants to pay z of Alice's debt and receive `1.1 * z` of her collateral. Also, consider Alice has a health factor under MIN_HEALT_FACTOR. We want to calculate what is the minimum amount that Alice must have as collateral to pay the full amount of debt as well as the liquidation bonus when she is getting liquidated. After liquidation, the collateral must at least be twice the debt:

(x - 1.1 \times z) \times 2 \leq y - z

In the previous equation, we are saying the amount that is going to be deducted from Alice's collateral in the liquidation process is 1.1 * z. Then, the collateral amount minus the deducted value must be twice as the debt minus the deducted value. The minimum amount happens when the left-hand side is equal to the right-hand side. So, we want to calculate the equation below:

2x - 2.2z = y - z

Also, for calculating the minimum amount, we have to assume that all of Alice's collateral can be liquidated now (z = x / 1.1). So, we change the equation to the equation below:

y = \frac{x}{1.1}

When the collateral is less than 1.1 times of the debt, Alice cannot pay the full amount to get liquidated. Hence, Alice is never going to be liquidated completely, unless her collateral becomes 1.1 times more than her debt. However, when, for example, the collateral is 1.05 times more than the debt, the liquidator still has incentives to liquidate the user and get a liquidation bonus.

This problem is more probable since this protocol can use multiple types of collateral. One collateral may crash and use its value, and the user's health factor reaches 100 to 110%. The liquidators should liquidate this user completely using the other collateral; however, this will not happen.

For example, consider Alice deposits 105 of WETH and 95 of WBTC. Also, Alice mints 100 of DSC. Now, their health factor is more than `MIN_HEALTH_FACTOR`. Now, consider WBTC crashes and its value reaches 0. Now, Alice has 105 of WETH collateral and 100 of DSC debt. Her health factor is way less than `MIN_HEALTH_FACTOR`. Also, she is over-collateralized. However, no liquidator can send 100 of DSC to receive 105 of WETH; however, they can send ~95 of DSC to receive 105$ of WETH. However, after that, the health factor is not going to be improved and the transaction is going to be reverted again. Hence, when the over-collateralized ratio is between 1 to 1.1, the user is never get liquidated.

Executed the test below:

function testCriticalHealthFactor() public {
    // Arranging the liquidator
    uint256 liquidatorCollateral = 10e18;
    ERC20Mock(weth).mint(liquidator, liquidatorCollateral);
    vm.startPrank(liquidator);
    ERC20Mock(weth).approve(address(dsce), liquidatorCollateral);
    uint256 liquidatorDebtToCover = 200e18;
    dsce.depositCollateralAndMintDsc(weth, liquidatorCollateral, amountToMint);
    dsc.approve(address(dsce), liquidatorDebtToCover);
    vm.stopPrank();

    // We set the price of WETH to $105 and WBTC to $95
    int256 wethUsdPrice = 105e8;
    MockV3Aggregator(ethUsdPriceFeed).updateAnswer(wethUsdPrice);
    int256 wbtcUsdPrice = 95e8;
    MockV3Aggregator(btcUsdPriceFeed).updateAnswer(wbtcUsdPrice);

    // Alice deposits 1 WBTC and 1 WETH and mints 100 DSC
    uint256 amountWethToDeposit = 1e18;
    uint256 amountWbtcToDeposit = 1e18;
    uint256 amountDscToMint = 100e18;
    vm.startPrank(user);
    ERC20Mock(weth).approve(address(dsce), amountWbtcToDeposit);
    dsce.depositCollateral(weth, amountWbtcToDeposit);
    ERC20Mock(wbtc).approve(address(dsce), amountWethToDeposit);
    dsce.depositCollateralAndMintDsc(wbtc, amountWethToDeposit, amountDscToMint);

    // WBTC crashes in its price will be $0
    int256 wbtcUsdPriceAfterCrash = 0;
    MockV3Aggregator(btcUsdPriceFeed).updateAnswer(wbtcUsdPriceAfterCrash);

    // Now, a liquidator tries to liquidate $100 of Alice's debt, and it will be reverted.
    vm.expectRevert();
    vm.startPrank(liquidator);
    dsce.liquidate(weth, user, amountDscToMint);
    vm.stopPrank();

    // The liquidator tries to liquidate $94.5 of Alice's debt, and it will be reverted.
    uint256 maxValueToLiquidate = 94.5e18;
    vm.expectRevert();
    vm.startPrank(liquidator);
    dsce.liquidate(weth, user, maxValueToLiquidate);
    vm.stopPrank();
}

Tools Used

Manual Review

Recommendations

When the health factor is between 100 to 110%, make the liquidation bonus to the maximum possible amount, not the fix amount of 1.1 * liqudationAmount. You can do that by adding the following code to the liquidate() function befre calling _redeemCollateral():

uint256 totalDepositedCollateral = s_collateralDeposited[user][collateral];
if (tokenAmountFromDebtCovered < totalDepositedCollateral && totalCollateralToRedeem > totalDepositedCollateral) {
    totalCollateralToRedeem = totalDepositedCollateral;
}