medium

`liquidate` does not allow the liquidator to liquidate a user if the liquidat...

Reward

Total

396.27 USDC

38.10 USDC
Selected
53.34 USDC
38.10 USDC
38.10 USDC
38.10 USDC
38.10 USDC
38.10 USDC
38.10 USDC
38.10 USDC
38.10 USDC
Selected Submission

liquidate does not allow the liquidator to liquidate a user if the liquidator HF < 1

Severity

Medium Risk

Relevant GitHub Links

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

Summary

The liquidate function does not allow the liquidator to liquidate the borrower if the liquidatorHF < 1.

By liquidating a user, the liquidator is using his own funds that do not impact the liquidator HF directly.

Because the function reverts, the system is preventing a user's to perform an action that should be able to do.

Vulnerability Details

The liquidate function does not allow the liquidator to liquidate the borrower if the liquidatorHF < 1.

By liquidating a user, the liquidator is using his own funds that do not impact the liquidator HF directly.

Because the function reverts, the system is preventing a user's to perform an action that should be able to do.

Impact

A liquidator cannot liquidate a user's debt when the liquidator's HF is below 1. The system is preventing a user to perform an action that does not impact his own HF.

Tools Used

Manual + foundry test

// SPDX-License-Identifier: MIT

pragma solidity 0.8.19;

import {DSCEngine} from "../../src/DSCEngine.sol";
import {DecentralizedStableCoin} from "../../src/DecentralizedStableCoin.sol";
import {HelperConfig} from "../../script/HelperConfig.s.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/ERC20Mock.sol";
import {Test, console} from "forge-std/Test.sol";
import {StdCheats} from "forge-std/StdCheats.sol";
import {MockV3Aggregator} from "../mocks/MockV3Aggregator.sol";

contract CannotLiquidateWhenHFTest is StdCheats, Test {
    DSCEngine public dsce;
    DecentralizedStableCoin public dsc;
    HelperConfig public helperConfig;

    address[] public tokenAddresses;
    address[] public priceFeedAddresses;

    address public ethUsdPriceFeed;
    address public btcUsdPriceFeed;
    address public weth;
    address public wbtc;
    uint256 public deployerKey;

    uint256 amountCollateral = 10 ether;
    uint256 amountToMint = 100 ether;
    address public user = address(1);

    uint256 public constant STARTING_USER_BALANCE = 10 ether;
    uint256 public constant MIN_HEALTH_FACTOR = 1e18;
    uint256 public constant LIQUIDATION_THRESHOLD = 50;


    function setUp() external {
        helperConfig = new HelperConfig();

        (ethUsdPriceFeed, btcUsdPriceFeed, weth, wbtc, deployerKey) = helperConfig.activeNetworkConfig();

        tokenAddresses = [weth, wbtc];
        priceFeedAddresses = [ethUsdPriceFeed, btcUsdPriceFeed];

        dsc = new DecentralizedStableCoin();
        dsce = new DSCEngine(tokenAddresses, priceFeedAddresses, address(dsc));

        dsc.transferOwnership(address(dsce));

        if (block.chainid == 31337) {
            vm.deal(user, STARTING_USER_BALANCE);
        }
        
        ERC20Mock(weth).mint(user, STARTING_USER_BALANCE);
        ERC20Mock(wbtc).mint(user, STARTING_USER_BALANCE);
    }

function testLiquidateRevertIfLiquidatorHFBelow() public {
        vm.startPrank(user);
        ERC20Mock(weth).approve(address(dsce), amountCollateral);
        dsce.depositCollateralAndMintDsc(weth, amountCollateral, amountToMint);
        vm.stopPrank();

        // liquidator 
        address liquidator = makeAddr("liquidator");
        ERC20Mock(weth).mint(liquidator, STARTING_USER_BALANCE);

        vm.startPrank(liquidator);
        ERC20Mock(weth).approve(address(dsce), amountCollateral);
        dsce.depositCollateralAndMintDsc(weth, amountCollateral, amountToMint);
        vm.stopPrank();

        // now let's say that price goes down
        int256 ethUsdUpdatedPrice = 18e8; // 1 ETH = $18

        MockV3Aggregator(ethUsdPriceFeed).updateAnswer(ethUsdUpdatedPrice);
        assertLt(dsce.getHealthFactor(user), 1e18);
        assertLt(dsce.getHealthFactor(liquidator), 1e18);


        // Liquidator try to liquidate 1 wei of user's debt but it will revert because of the check
        vm.startPrank(liquidator);
        dsc.approve(address(dsce), 1 ether);

        // system revert because `liquidator` has HF < 1
        vm.expectRevert();
        dsce.liquidate(weth, user, 1 ether);
        vm.stopPrank();


        vm.startPrank(liquidator);

        // Liquidator supply 1000 ether and supply them to have HF > 1
        ERC20Mock(weth).mint(liquidator, 1000 ether);
        ERC20Mock(weth).approve(address(dsce), 1000 ether);
        dsce.depositCollateral(weth, 1000 ether);

        
        uint256 liquidatorHFBefore = dsce.getHealthFactor(liquidator);
        assertGe(liquidatorHFBefore, 1e18);

        // perform liquidation again and prove that HF of liquidator does not change because of the liquidation itself
        dsc.approve(address(dsce), 1 ether);
        dsce.liquidate(weth, user, 1 ether);

        // The liquidator is using his own funds that do not impact the liquidator HF
        assertEq(dsce.getHealthFactor(liquidator), liquidatorHFBefore);
        vm.stopPrank();
    }
}

Recommendations

The system should remove the check _revertIfHealthFactorIsBroken(msg.sender); in the liquidate() function, allowing a liquidator to always be able to liquidate a borrower.