DSCEngine::liquidate doesn't enforce restoration of the Health Factor for liquidated users

Severity

Medium Risk

Relevant GitHub Links

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

Summary

Currently, liquidators can choose how much debt they want to cover from an under-collateralized user in DSCEngine::liquidate, but the amount they might decide to cover may not restore the health factor to be equal or greater than MIN_HEALTH_FACTOR

Vulnerability Details

Assume DSCEngine.sol and DecentralizedStableCoin.sol are deployed and a user decides to use the protocol.

The user deposits 10 WBTC tokens in the protocol as collateral and chooses to mint 4,000 DSC tokens.
The price of each WBTC token is of $1,000, therefore the total value of the user collateral in USD is of $10,000.
The price of WBTC tanks to $750 per token, decreasing the value of the collateral to $7,500.
Due to the decrease of the price of WBTC the health factor of the user also decreases from 1.25 to 0.9375.
The user is now under-collateralized.

A liquidator decides to partially liquidate the user to obtain a portion of the user collateral at a discount. Instead of paying the full debt of 4,000 DSC, he chooses to only cover 500 DSC.

    function test_partiallyLiquidatesUser() public {
        vm.startPrank(liquidator);
        ERC20Mock(weth).mint(liquidator, 20000 ether);
        ERC20Mock(weth).approve(address(dsce), type(uint256).max);
        dsce.depositCollateralAndMintDsc(weth, 20000 ether, 20000 ether);
        dsc.approve(address(dsce), type(uint256).max);
        vm.stopPrank();

        vm.startPrank(user);
        ERC20Mock(wbtc).mint(user, 10 ether);
        ERC20Mock(wbtc).approve(address(dsce), type(uint256).max);
        dsce.depositCollateralAndMintDsc(wbtc, 10 ether, 4000 ether);
        vm.stopPrank();

        console.log("\n", "\t", "User health factor before price update:", dsce.getHealthFactor(user));

        MockV3Aggregator(wbtcUsdPriceFeed).updateAnswer(750e8);

        console.log("\t", "User health factor before liquidation:", dsce.getHealthFactor(user));

        vm.prank(liquidator);
        dsce.liquidate(wbtc, user, 500 ether);

        console.log("\t", "User health factor after liquidation:", dsce.getHealthFactor(user), "\n");
    }

The output is shown below.

Running 1 test for test/myTests/unit/DSCEngineTest.t.sol:DSCEngineTest
[PASS] test_partiallyLiquidatesUser() (gas: 593421)
Logs:

         User health factor before price update: 1250000000000000000
         User health factor before liquidation: 937500000000000000
         User health factor after liquidation: 992857142857142857

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 50.79ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

The protocol aims to be 200% over-collateralized and solvent at all times, by allowing under-collateralized users to remain in the system after a liquidation represents a threat to the solvency and avaliability of the protocol. For these reasons I evaluate the severity to MEDIUM.

Tools Used

Visual Studio code and Foundry.

Recommendations

Consider modifying the sanity check after a liquidation to revert if the liquidated user health factor is not equal or greater than MIN_HEALTH_FACTOR.

Cyfrin / 2023-07-foundry-defi-stablecoin