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

DSCEngine.sol#liquidate() - A user can liquidate himself, effectively not allowing another liquidator to take rewards from his liquidation

0xdeth

Summary

A user can liquidate himself, effectively not allowing another liquidator to take rewards from his liquidation.

Vulnerability Details

Imagine the following scenario. Alice deposits collateral and mints dsc. Her initial health factor is okay and she isn't at risk of being liquidated. After some time, the price of Alice's collateral drops and her position is now eligible for liquidation. Alice sees that she can be liquidated and she liquidates herself. She takes no reward, but she takes the chance for someone else, another liquidator, to take the rewards for her liquidation since she is already liquidated.

POC:

function testFailLiquidationIfUserLiquidatesHimselfAndThemSomeoneElseTriesToLiquidateHim() public {
// User deposits collateral and mints DSC.
vm.startPrank(user);
ERC20Mock(weth).approve(address(dsce), amountCollateral);
dsce.depositCollateralAndMintDsc(weth, amountCollateral, amountToMint);
vm.stopPrank();
int256 ethUsdUpdatedPrice = 18e8; // 1 ETH = $18
​
// Price of the collateral drops and now User is eligible for liquidation
MockV3Aggregator(ethUsdPriceFeed).updateAnswer(ethUsdUpdatedPrice);
// User liquidates himself, not taking any reward, but also takes the chance for Liquidator
// or any other liquidator to take the reward for liquidating User
vm.startPrank(user);
dsc.approve(address(dsce), amountToMint);
dsce.liquidate(weth, user, amountToMint);
vm.stopPrank();
​
// Liquidator attempts to liquidate User, but his whole debt has already been covered
// and so his health factor is ok.
ERC20Mock(weth).mint(liquidator, collateralToCover);
vm.startPrank(liquidator);
ERC20Mock(weth).approve(address(dsce), collateralToCover);
dsce.depositCollateralAndMintDsc(weth, collateralToCover, amountToMint);
dsc.approve(address(dsce), amountToMint);
dsce.liquidate(weth, user, amountToMint);
vm.stopPrank();
​
vm.expectRevert(DSCEngine.DSCEngine__HealthFactorOk.selector);
}

Impact

A user that liquidates himself takes the chance for another user to reap the rewards for his liquidation.

Tools Used

Manual Review
Foundry

Recommendations

Add a check inside the liquidate() function that checks if the user that was passed is different from msg.sender

function liquidate(address collateral, address user, uint256 debtToCover)
external
moreThanZero(debtToCover)
nonReentrant
{
// Add some sort of check in the beginning of the function that checks if user == msg.sender
require(user != msg.sender, "A user cannot liquidate themselves");
// need to check health factor of the user
uint256 startingUserHealthFactor = _healthFactor(user);
if (startingUserHealthFactor >= MIN_HEALTH_FACTOR) {
revert DSCEngine__HealthFactorOk();
}
// We want to burn their DSC "debt"
// And take their collateral
// Bad User: $140 ETH, $100 DSC
// debtToCover = $100
// $100 of DSC == ??? ETH?
// 0.05 ETH
uint256 tokenAmountFromDebtCovered = getTokenAmountFromUsd(collateral, debtToCover);
// And give them a 10% bonus
// So we are giving the liquidator $110 of WETH for 100 DSC
// We should implement a feature to liquidate in the event the protocol is insolvent
// And sweep extra amounts into a treasury
// 0.05 * 0.1 = 0.005. Getting 0.055
uint256 bonusCollateral = (tokenAmountFromDebtCovered * LIQUIDATION_BONUS) / LIQUIDATION_PRECISION;
uint256 totalCollateralToRedeem = tokenAmountFromDebtCovered + bonusCollateral;
_redeemCollateral(user, msg.sender, collateral, totalCollateralToRedeem);
// We need to burn the DSC
_burnDsc(debtToCover, user, msg.sender);
​
uint256 endingUserHealthFactor = _healthFactor(user);
if (endingUserHealthFactor <= startingUserHealthFactor) {
revert DSCEngine__HealthFactorNotImproved();
}
_revertIfHealthFactorIsBroken(msg.sender);
}

Support

FAQs

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