Skip to content

Latest commit

 

History

History
157 lines (119 loc) · 8.62 KB

080.md

File metadata and controls

157 lines (119 loc) · 8.62 KB

Orbiting Sangria Porpoise

High

Debts can never be fully liquidated due to closeFactorMantissa constraint

Summary

A debt can only be partially liquidated due to the closeFactorMantissa & maxClose constraint. The remaining balance remains stuck in the system as unhealthy debt.

Root Causes

Description

  • Although the tests have been setup with a closeFactorMantissa of 1, this is not as per specs of the protocol as can be seen here:
        // closeFactorMantissa must be strictly greater than this value
        uint internal constant closeFactorMinMantissa = 0.05e18; // 0.05

@-->    // closeFactorMantissa must not exceed this value
        uint internal constant closeFactorMaxMantissa = 0.9e18; // 0.9

Currently _setCloseFactor() has a bug which allows surpassing this limit, but assuming that to not be the case in the live version, we assume a suitable value for closeFactorMantissa say, 0.9e18 or 90%.

  • This means whatever repayAmount is passed by the liquidator, it should not exceed 90% of the borrowBalance

  • The protocol allows only full liquidations if the borrowBalance goes below minBorrowAmountAllowPartialLiquidation which is currently set to 10 ether. This however is not working either due to the closeFactorMantissa & maxClose constraint.

As a result, there is no way to fully liquidate a debt with a shortfall once it goes below minBorrowAmountAllowPartialLiquidation.

Impact

This is highly problematic because this would occur for all the debts with a shortfall which go below minBorrowAmountAllowPartialLiquidation before attaining a healthy state via partial liquidations. Consider the following:

  • Setup: Acceptable LTV is 80% and current LTV is 90%. borrowBalance = 20 ether. Eligible for liquidation.

  • First liquidation call : Partial liquidation improves the LTV to 85% but now borrowBalance = 9 ether i.e. below minBorrowAmountAllowPartialLiquidation.

  • Second liquidation call: Only full liquidation allowed now. But even if the liquidator passes type(uint256).max as the repay amount, it will be reduced due to closeFactorMantissa < 1 and hence call will revert with error TOO_MUCH_REPAY.

Proof of Concept

The following test shows how a shortfall-debt which was initially above minBorrowAmountAllowPartialLiquidation eventually transitioned into borrowBalance < minBorrowAmountAllowPartialLiquidation & could not be fully liquidated at any stage:

  • Add the necessary imports inside Vault.t.sol first:
    import {ComptrollerErrorReporter, TokenErrorReporter} from "../lending/ErrorReporter.sol";
  • Then add the following test and run to see it pass:
    function test_cannotLiquidateFully() public {
        // Initial setup
        vm.startPrank(deployer);
        vaultManager.setSellFee(1 ether); // no sell fee
        comptroller._setCollateralFactor(cNuma, 0.85 ether); // 85% LTV allowed
        vm.stopPrank();

        uint collateralAmount = 25 ether;
        uint borrowAmount = 20 ether;  // 80% LTV to start

        deal({token: address(rEth), to: userA, give: collateralAmount});
        
        // First approve and enter the NUMA market
        vm.startPrank(userA);
        address[] memory t = new address[](1);
        t[0] = address(cNuma);
        comptroller.enterMarkets(t);

        // Buy NUMA with rETH to use as collateral
        rEth.approve(address(vault), collateralAmount);
        uint numas = vault.buy(collateralAmount, 0, userA);

        // Deposit NUMA as collateral
        uint cNumaBefore = cNuma.balanceOf(userA);
        numa.approve(address(cNuma), numas);
        cNuma.mint(numas);
        uint cNumas = cNuma.balanceOf(userA) - cNumaBefore;
        
        emit log_named_decimal_uint("Numas deposited =", numas, 18);
        emit log_named_decimal_uint("cNumas received =", cNumas, 18);

        // Borrow rETH
        cReth.borrow(borrowAmount);
        uint initialBorrowBalance = cReth.borrowBalanceCurrent(userA);
        emit log_named_decimal_uint("Initial borrow =", initialBorrowBalance, 18);
        (, , uint shortfall, uint badDebt) = comptroller.getAccountLiquidityIsolate(userA, cNuma, cReth);
        assertEq(shortfall, 0, "Unhealthy borrow");
        vm.stopPrank();

        // Make position liquidatable
        vm.startPrank(deployer);
        vaultManager.setSellFee(0.90 ether); 
        
        // Verify position is liquidatable
        (, , shortfall, badDebt) = comptroller.getAccountLiquidityIsolate(userA, cNuma, cReth);
        assertGt(shortfall, 0, "Position should be liquidatable");
        assertEq(badDebt, 0, "Position shouldn't be in badDebt region");
        emit log_named_decimal_uint("Shortfall =", shortfall, 18);

        // Set liquidation incentive
        comptroller._setLiquidationIncentive(1.05e18); // 5% liquidation incentive
        // Set close factor
        comptroller._setCloseFactor(0.9e18); // 90%
        vm.stopPrank();

        // First liquidation attempt
        vm.startPrank(userC); // liquidator
        uint borrowBalance = cReth.borrowBalanceCurrent(userA);
        uint repayAmount = (borrowBalance * 55) / 100; // repaying 55% of the debt
        
        deal({token: address(rEth), to: userC, give: repayAmount});
        rEth.approve(address(vault), repayAmount);
        
        vault.liquidateLstBorrower(userA, repayAmount, false, false);
        emit log_named_decimal_uint("First liquidation repaid =", repayAmount, 18);

        uint remainingBorrow = cReth.borrowBalanceCurrent(userA);
        emit log_named_decimal_uint("Remaining borrow =", remainingBorrow, 18);
        // Only full liquidation allowed now
        assertLt(remainingBorrow, vault.minBorrowAmountAllowPartialLiquidation(), "below minBorrowAmountAllowPartialLiquidation");
        // Verify again the position is liquidatable but is not in the badDebt region
        (, , shortfall, badDebt) = comptroller.getAccountLiquidityIsolate(userA, cNuma, cReth);
        emit log_named_decimal_uint("Shortfall2 =", shortfall, 18);
        emit log_named_decimal_uint("BadDebt2   =", badDebt, 18);
        assertGt(shortfall, 0, "Position2 should be liquidatable");
        assertEq(badDebt, 0, "Position2 shouldn't be in badDebt region");

        deal({token: address(rEth), to: userC, give: remainingBorrow});
        uint repay = remainingBorrow; // full repayment
        rEth.approve(address(vault), repay);

        // @audit-issue : full repay not allowed
        vm.expectRevert(abi.encodeWithSelector(TokenErrorReporter.LiquidateComptrollerRejection.selector, uint(ComptrollerErrorReporter.Error.TOO_MUCH_REPAY)));
        vault.liquidateLstBorrower(userA, repay, false, false); 
        
        // @audit-issue : partial repay not allowed too 
        vm.expectRevert("min liquidation");
        vault.liquidateLstBorrower(userA, repay * 8 / 10, false, false); 
        vm.stopPrank();
        
        // @audit-info : Let's also check that IF the bug didn't exist and full liquidation was
        // allowed, whether or not it would've went through successfully?
        vm.prank(deployer);
        comptroller._setCloseFactor(1e18); // temporary hack to verify intended behaviour

        vm.prank(userC); // liquidator
        vault.liquidateLstBorrower(userA, repay, false, false); // should pass
    }

Mitigation

One approach would be to ignore the _setCloseFactor calculation under following circumstances:

  • when borrowBalance < minBorrowAmountAllowPartialLiquidation.
  • (optional) when user opts for a full liquidation.

This means that the protocol should consider maxClose to be equal to borrowBalance in the above scenarios.