Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Liquidation of a user might not be possible if their collateral amount is less than the amount required for the bonus award

neocrao

Summary

Liquidation of a user might not be possible if their collateral amount is less than the amount required for the bonus award

Vulnerability Details

In the liquidate() method, the user who is liquidating another user gets a collateral bonus of 10%, making a total return of 110%.

If the user thats getting liquidated doesn't have enough collateral to cover 110%, then the liquidation may not be possible for the user.
Attempting this causes an arithmetic underflow, as the collateral balance of the user that's getting liquidated goes into negative.

​
229: function liquidate(address collateral, address user, uint256 debtToCover)
​

Link to code - https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L229

Proof of Concept

The unit test below demonstrates the issue. The flow is as follows:

  • Asusme that 1 ETH price is 2000 USD.

  • [UserToBeLiquidated] Starts by minting 2000 DSC, with a collateral of 2 ETH, which is 4000 USD.

  • [UserToBeLiquidated] Their health factor is 1, so its all good.

  • Now, 1 ETH drops 1000 USD.

  • [UserToBeLiquidated] holds 2000 DSC, with a collateral of 2 ETH, which is only 2000 USD now.

  • [UserToBeLiquidated] Their health factor is 0.5, so they can be liquidated

  • [LiquidatorUser] They try to liquidate UserToBeLiquidated by covering their complete debt of 2000 DSC

  • [LiquidatorUser] The method liquidate() is called with ETH as the collateral token, UserToBeLiquidated as the user, and 2000 DSC as the debt to cover

  • Now, In the liquidate() method, the following happens:

    • getTokenAmountFromUsd() method tries to get the value of ETH collateral amount for 2000 DSC. As DSC is stablecoin, its equivalent to 2000 USD. With the current rate of 1 ETH = 1000 USD, the method returns 2 ETH as the collateral token amount.

    • To 2 ETH, a bonus of 10% is added next. This makes the total reward for the user as 2.2 ETH.

    • Later in the method, _redeemCollateral() method is called, where the collateral balance of UserToBeLiquidated is attempted to reduce by 2.2 ETH. But the current collateral balance of UserToBeLiquidated is only 2 ETH. This causes an undeflow, and so the error.

Steps to run this:

  • Put the test in DSCEngineTest.t.sol

  • Execute the test using the command forge test --match-test testLiquidationGetsTooMuchCollateralWithBonus -vv

function testLiquidationGetsTooMuchCollateralWithBonus() public {
uint256 amountCollateral = 2 ether;
uint256 amountToMint = 2000 ether;
​
// Prepare the user that will be liquidated
vm.startPrank(user);
ERC20Mock(weth).approve(address(dsce), amountCollateral);
dsce.depositCollateralAndMintDsc(weth, amountCollateral, amountToMint);
vm.stopPrank();
​
ERC20Mock(weth).mint(liquidator, collateralToCover);
​
// Prepare the liquidator
vm.startPrank(liquidator);
ERC20Mock(weth).approve(address(dsce), collateralToCover);
dsce.depositCollateralAndMintDsc(weth, collateralToCover, amountToMint);
dsc.approve(address(dsce), amountToMint);
vm.stopPrank();
​
// 2 | 000 000 000 000 000 000 // ETH token count
console.log("[User to be Liquidated] amountCollateral before ETH price drop");
console.log(amountCollateral);
// 100 | 000 000 000 000 000 000
// 2000 | 000 000 000 000 000 000 // DSC token count
console.log("[User to be Liquidated] amountToMint before ETH price drop");
console.log(amountToMint);
​
// 1 | 000 000 000 000 000 000 // Health
console.log("[User to be Liquidated] Health before ETH price drop");
console.log(dsce.getHealthFactor(user));
// 4000 | 000 000 000 000 000 000 // Collateral in USD
console.log("[User to be Liquidated] Collateral value in USD before ETH price drop");
console.log(dsce.getAccountCollateralValue(user));
​
int256 ethUsdUpdatedPrice = 1000e8; // 1 ETH = $1000
MockV3Aggregator(ethUsdPriceFeed).updateAnswer(ethUsdUpdatedPrice);
​
uint256 userHealthFactor = dsce.getHealthFactor(user);
​
// 0.5 | 000 000 000 000 000 000 // Health
console.log("[User to be Liquidated] Health after ETH price drop");
console.log(userHealthFactor);
// 2000 | 000 000 000 000 000 000 // Collateral in USD
console.log("[User to be Liquidated] Collateral value in USD after ETH price drop");
console.log(dsce.getAccountCollateralValue(user));
​
vm.startPrank(liquidator);
vm.expectRevert(stdError.arithmeticError);
dsce.liquidate(weth, user, amountToMint); // We are covering their whole debt
vm.stopPrank();
​
}

Severity Justification

Marking this as High as both the following High criteria satisfy:

  • Funds are directly or nearly directly at risk

Source: https://docs.codehawks.com/rewards-and-judging

Tools Used

Manual inspection

Recommendations

There are several options:

  • Add a check if the bonus can be awarded to the user.

  • Let the user choose if they are fine to liquidate the user without a reward, or make it a configurable reward thats supplied as an input to the liquidate method.

  • Make it a protocol rule to reward a bonus only if its possible to cover it.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.