Summary

When a user liquidates a position, they receive a bonus of 10%. Despite this incentive, the amount of collateral that becomes locked in the pool exceeds the total amount received for the liquidation, making this process financially unfavorable.

Vulnerability Details

When a position is subject to liquidation, the liquidator's balance experiences a deduction equal to the amount of debt to cover, which is attainable through the invocation of the function balanceOf(). In contrast, the mapping s_DSCMinted remains unchanged, retaining the original values. Consequently, the liquidator has to keep a quantity of collateral held. The amount of collateral held exceeds the sum obtained through the liquidation process, resulting in a financial loss for the liquidator. This collateral becomes immobilized preventing the user from redeeming it. The only way of recovering some of that collateral is to go undercollateralized and get liquidated.

Impact

Let's consider this scenario:
We have two users who deposit collateral and mint some DSC. The initial quotation price of ETH/USD is $2,000.

• User_1 deposits 10 WETH and mints the maximum amount of DSC allowed (i.e. 10,000 DSC)

• User_2 deposits 20 WETH and mints the same amount as User_1 (10,000 DSC)

Now, suppose the quotation price of ETH/USD falls to $1,800. In this scenario, User_1 collateral falls below the required threshold being susceptible to liquidation. User_2 still has a Health Factor above 1.
User_2 proceeds to initiate a complete liquidation of User_1's position, resulting in the following balances for each user:

• User_1 => 3.8888 WETH and 0 DSC

• User_2 => 20 WETH and 10,000 DSC. Additionally, user_2 gets 6.1111 WETH transferred as payment for the liquidation.

Subsequently, if User_2 decides to redeem all collateral for DSC, User_2 ends up with a balance of 15 WETH because 11.11111 WETH are held as collateral. User_2 started with a balance of 20 WETH and after liquidating user_1 and redeeming all collateral, the final balance is 15 WETH making liquidation not financially favorable. Precisely because balanceOf() provides the DSC amount after deducting the amount liquidated, while s_DSCMinted contains the original amount, the liquidator has to have collateral to cover the amount contained within s_DSCMinted but do not have access to those tokens.

Tools Used

Foundry

// SPDX-License-Identifier: MIT

pragma solidity 0.8.19;

import "forge-std/Test.sol";
import "../../src/DSCEngine.sol";
import "../../src/DecentralizedStableCoin.sol";
import {MockV3Aggregator} from "../mocks/MockV3Aggregator.sol";
import {AggregatorV3Interface} from "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import {OracleLib} from "../../src/libraries/OracleLib.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/ERC20Mock.sol";

contract TestBalanceOfDSC is Test {
    using OracleLib for AggregatorV3Interface;

    uint8 public constant DECIMALS = 8;
    int256 public constant ETH_USD_INITIAL_PRICE = 2000e8;
    int256 public constant ETH_USD_EVOLVED_PRICE = 1800e8;
    int256 public constant BTC_USD_INITIAL_PRICE = 1000e8;
    int256 public constant BTC_USD_EVOLVED_PRICE = 999e8;

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

    DSCEngine public dsce;
    DecentralizedStableCoin public dsc;

    address public weth;
    address public wbtc;

    address public owner = makeAddr("owner");
    address public user_1 = makeAddr("user_1");
    address public user_2 = makeAddr("user_2");
    address public user_3 = makeAddr("user_3");

    MockV3Aggregator public ethUsdPriceFeed;
    MockV3Aggregator public btcUsdPriceFeed;
    ERC20Mock public wethMock;
    ERC20Mock public wbtcMock;

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

    function setUp() public {
        vm.startPrank(owner);
        dsc = new DecentralizedStableCoin();
        ethUsdPriceFeed = new MockV3Aggregator(DECIMALS, ETH_USD_INITIAL_PRICE);
        wethMock = new ERC20Mock("WETH", "WETH", msg.sender, 1000e8);
        btcUsdPriceFeed = new MockV3Aggregator(DECIMALS, BTC_USD_INITIAL_PRICE);
        wbtcMock = new ERC20Mock("WBTC", "WBTC", msg.sender, 1000e8);

        weth = address(wethMock);
        wbtc = address(wbtcMock);

        tokenAddresses = [weth, wbtc];
        priceFeedAddresses = [address(ethUsdPriceFeed), address(btcUsdPriceFeed)];

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

        wethMock.mint(user_1, STARTING_USER_BALANCE);
        wethMock.mint(user_2, STARTING_USER_BALANCE * 2);
        wethMock.mint(user_3, STARTING_USER_BALANCE * 5);
        vm.stopPrank();

        assertEq(wethMock.balanceOf(user_1), 10 ether);
        assertEq(wethMock.balanceOf(user_2), 20 ether);
        assertEq(wethMock.balanceOf(user_3), 50 ether);
}

    function testCollateralHeldLargerThanReward() public {
        //assert initial balances for user_1 and user_2
        uint256 user_1InitialBalanceofWeth = wethMock.balanceOf(user_1);
        uint256 user_2InitialBalanceofWeth = wethMock.balanceOf(user_2);
        assertEq(user_1InitialBalanceofWeth, 10 ether);
        assertEq(user_2InitialBalanceofWeth, 20 ether);

        // user_1 deposits 10 weth and mints the maximum amount of DSC => 10,000 DSC
        vm.startPrank(user_1);
        wethMock.approve(address(dsce), 10 ether);
        dsce.depositCollateralAndMintDsc(address(weth), 10 ether, 10_000 ether);
        vm.stopPrank();

        // user_2 deposits 20 weth and mints 10,000 DSC
        vm.startPrank(user_2);
        wethMock.approve(address(dsce), 20 ether);
        dsce.depositCollateralAndMintDsc(address(weth), 20 ether, 10_000 ether);
        vm.stopPrank();

        // Health Factor is calculated before the price of weth is updated
        // TotalDscMinted is calculated before the position is liquidated
        uint256 user_1HealthFactorBeforePriceUpdate = dsce.getHealthFactor(user_1);
        (uint256 user_1BeforeLiquidationTotalDscMinted,) = dsce.getAccountInformation(user_1);
        uint256 user_2HealthFactorBeforePriceUpdate = dsce.getHealthFactor(user_2);
        (uint256 user_2BeforeLiquidationTotalDscMinted,) = dsce.getAccountInformation(user_2);

        assertGe(user_1HealthFactorBeforePriceUpdate, 1 ether);
        assertGe(user_2HealthFactorBeforePriceUpdate, 1 ether);
        assertEq(user_1BeforeLiquidationTotalDscMinted, 10_000 ether);
        assertEq(user_2BeforeLiquidationTotalDscMinted, 10_000 ether);
        assertEq(dsc.balanceOf(user_1), user_1BeforeLiquidationTotalDscMinted);
        assertEq(dsc.balanceOf(user_2), user_2BeforeLiquidationTotalDscMinted);

        // the price of weth is updated to $1,800 making user_1 go undercollateralized
        ethUsdPriceFeed.updateRoundData(2, ETH_USD_EVOLVED_PRICE, block.timestamp, 2);
        uint256 wethPrice = dsce.getUsdValue(weth, 1 ether);
        assertEq(wethPrice, 1_800e18);

        // Health Factor is assessed after the price of weth is updated to make sure user_1 is undercollateralized
        uint256 user_1HealthFactorAfterPriceUpdate = dsce.getHealthFactor(user_1);
        assertLt(user_1HealthFactorAfterPriceUpdate, 1 ether);

        // user_2 gets user_1 fully liquidated
        vm.startPrank(user_2);
        dsc.approve(address(dsce), 10_000 ether);
        dsce.liquidate(weth, user_1, 10_000 ether);
        vm.stopPrank();

        // assert user_2 balances of DSC after liquidation
        (uint256 user_2AfterLiquidationTotalDscMinted,) = dsce.getAccountInformation(user_2);
        assertEq(user_2AfterLiquidationTotalDscMinted, 10_000 ether);
        assertEq(dsc.balanceOf(user_2), 0);

        // user_2 redeems all collateral
        // since user_2 has to leave some collateral to cover 10,000 DSC, let's calculate the maximum amount that can get
        // redeemed without reverting because health factor gets broken
        // 10,000 DCS at a price of ETH/USD of 1,800 equals 5.55555 WETH. If we apply the liquidation threshold, it equals 11.11111 WETH
        // let's calculate the health factor of 10,000 DSC at ETH/USD $1,800 with a collateral amount of 11.1111 WETH
        // (11.11111 * 1,800 * 50 / 100) / 10,000 = 1. So 11.1111111 WETH is the minimum amount of collateral to be held.
        // The amount to be redeemed then is 20 - 11.111111 = 8.8888888888.
        vm.startPrank(user_2);
        dsce.redeemCollateral(weth, 88888888888e8);
        vm.stopPrank();

        //assert maximum amount of collateral has been redeemed by checking health factor is equal to 1
        // Health Factor is not going to be exactly 1 so checking is less than 1.00000000001
        uint256 user_2HealthFactorAfterRedeem = dsce.getHealthFactor(user_2);
        assertLt(user_2HealthFactorAfterRedeem, 100000000001e7);

        // let's try to redeem more collateral on top of the amount already redeemed and see if reverts
        vm.startPrank(user_2);
        vm.expectRevert();
        dsce.redeemCollateral(weth, 1 ether);
        vm.stopPrank();

        // let's also try to redeem collateral for DSC and see if reverts
        vm.startPrank(user_2);
        dsc.approve(address(dsce), 1_800 ether);
        vm.expectRevert();
        dsce.redeemCollateralForDsc(weth, 1 ether, 1_800 ether);
        vm.stopPrank();

        // let's assert balance of user_2 before minting is greater than after redeeming
        uint256 user_2FinalBalanceofWeth = wethMock.balanceOf(user_2);
        assertGt(user_2InitialBalanceofWeth, user_2FinalBalanceofWeth);

        console.log("***************************************** RESULTS ***************************************");
        console.log("user_2 balance of WETH before deposit and mint =>  ", user_2InitialBalanceofWeth);
        console.log("user_2 balance of WETH after redeeming =>  ", user_2FinalBalanceofWeth);
        console.log("*****************************************************************************************");
}

Recommendations

The current behavior of this contract appears to impose a penalty on individuals liquidating a position, and it remains uncertain whether this outcome aligns with the intended functionality. If the observed behavior is unintended, a comprehensive analysis and redesign of the contract may be necessary to rectify the situation and align it with the intended objectives.