Foundry DeFi Stablecoin CodeHawks Audit Contest

Summary

A user whose _healthFactor is less than the MIN_HEALTH_FACTOR can liquidate themselves by setting the address of user to itself. This way, the user becomes overcollaterized and also earns more money/asset.

Vulnerability Details

Consider the following scenario :

1. Lets suppose initially the price of 1 eth is 2,000 usd. Alice calls depositCollateralAndMintDsc function and deposits 1 weth as collateral and mints 900 dsc. The _healthFactor of Alice is 1.11e18 that is greater than the MIN_HEALTH_FACTOR which is 1e18 so, Alice is overcollaterized.

2. Lets say after some time, price of ether goes down and the price of 1 eth is now 1750 usd. So, Alice’s collateral value goes down. Alice has 1750 usd backing 900 dsc that is not good and she is undercollaterized as her _healthFactor is now 0.977e18 that is less than MIN_HEALTH_FACTOR. Therefore, anyone can now liquidate her.

3. Alice notices she is undercollaterized and she decides to liquidate herself. She calls the liquidate function with address of user set to her address and sets 900 dsc as the debtToCover. Her tokenAmountFromDebtCovered is 0.514 eth and bonusCollateral is 0.0514 eth and her totalCollateralToRedeem becomes 0.5654 eth( approx. 989.45 usd). Now, _redeemCollateral function is called and s_collateralDeposited[Alice’s address][weth address] = 1 - 0.5654 = 0.4346 eth. Then _burnDSC function is called and 900 dsc goes back to the DSCEngine contract.
   So, Alice now has 0.4346 eth as collateral and 0 dsc. Therefore, she becomes overcollaterized and her _healthFactor is not broken.

4. Eventually, Alice utilized the 900 dsc that was initially minted, and earned 989.45 dsc after liquidation and has 0.4346 eth( 760.55 usd) as collateral. Therefore in total she has assets worth 2650 usd and therefore she has earned an additional 650 usd even after the price of eth went down.

Instance:

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L229C1-L262C6

function liquidate(address collateral, address user, uint256 debtToCover)
    external
    moreThanZero(debtToCover)
    nonReentrant
{   require( msg.sender != user ,"you cannot liquidate yourself");
    // 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);
}

Impact

Users exploit the system by extracting more assets than the collateral deposited.

Tools Used

Manual review and VS Code

Recommendations

Users should not be allowed to liquidate themselves. The following require statement can be implemented in the liquidate function.

require( msg.sender != user ,"you cannot liquidate yourself");