ArmedGoose - Liquidatable collateral may be delisted leading to bad debt

[sherlock-admin]

ArmedGoose

medium

Liquidatable collateral may be delisted leading to bad debt

Summary

If a market is also configured as a collateral, it can be hard delisted without any check if currently any borrowed positions are collateralized using that market. Due to that, after such delisting, there may still remain positions that will become unliquidatable. The tokens will remain in the protocol, but there might not be any possibility to liquidate them afterwards, so in the end, any potential debt will have to be absorbed by the protocol.

Vulnerability Detail

The vulnerability consists of several factors. Highlighting some protocol assumptions that lead to it:

• a Market can be configured as collateral: MarketConfigurator.sol
• a Market can be delisted: MarketConfigurator.sol
• For a liquidation to occur, collateral market has to be listed IronBank.sol

There is no actual check on delisting, if a market is currently a collateral. It may happen, that if a collateral market is delisted, then the position cannot be liquidated anymore. If the user won't repay, then there will be no way to liquidate that position.

Impact

Protocol may incur losses due to positions at loss which will not be liquidated. However, since delisting is on protocol side, it cannot be exploited by users on purpose, thus likelihood is low. I rate this in turn as Medium.

Code Snippet

As per below unit test, you can see user has an active debt, the market has been delisted, and there is now no possibility to liquidate that user. Even though it is mathematically liquidatable, it reverts due to "collateral not listed" as per the vm.expectRevert("collateral market not listed");

Run with forge test -vv --match-test test_liquidation_delisting_poc

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "./Common.t.sol";

contract LiquidateTest is Test, Common {
    uint16 internal constant reserveFactor = 1000; // 10%
    int256 internal constant market1Price = 1500e8;
    int256 internal constant market2Price = 200e8;
    uint16 internal constant market1CollateralFactor = 8000; // 80%
    uint16 internal constant market1LiquidationThreshold = 9000; // 90%
    uint16 internal constant market1LiquidationBonus = 11000; // 110%

    IronBank ib;
    MarketConfigurator configurator;
    CreditLimitManager creditLimitManager;
    TripleSlopeRateModel irm;
    FeedRegistry registry;
    PriceOracle oracle;

    ERC20Market market1;
    ERC20Market market2;

    address admin = address(64);
    address user1 = address(128);
    address user2 = address(256);

    function setUp() public {
        ib = createIronBank(admin); //deploy the contract

        configurator = createMarketConfigurator(admin, ib); //create market configurator

        vm.prank(admin);
        ib.setMarketConfigurator(address(configurator)); //set market configurator addr to control the Iron Bank

        creditLimitManager = createCreditLimitManager(admin, ib); //create CreditLimitManager role

        vm.prank(admin);
        ib.setCreditLimitManager(address(creditLimitManager)); //set CreditLimitManager role

        irm = createDefaultIRM(); //create default interest rate model

        (market1,,) = createAndListERC20Market(18, admin, ib, configurator, irm, reserveFactor); 
        (market2,,) = createAndListERC20Market(18, admin, ib, configurator, irm, reserveFactor); 

        registry = createRegistry();
        oracle = createPriceOracle(admin, address(registry)); //setup for using price oracles

        vm.prank(admin);
        ib.setPriceOracle(address(oracle)); //set the oracle

        setPriceForMarket(oracle, registry, admin, address(market1), address(market1), Denominations.USD, market1Price); 
        setPriceForMarket(oracle, registry, admin, address(market2), address(market2), Denominations.USD, market2Price); //set prices for the markets used in this test

        deal(address(market1), user1, 10000e18); //user 1 has decimals1 * market1 (10000e18)
        deal(address(market2), user2, 10000e18); //user 2 has decimals2 * market2

        vm.startPrank(admin);
        configurator.configureMarketAsCollateral(
            address(market1), market1CollateralFactor, market1LiquidationThreshold, market1LiquidationBonus
        );//set  params for market1 

        // Inject some liquidity for borrow.
        market2.approve(address(ib), type(uint256).max);
        ib.supply(admin, admin, address(market2), 10_000e18);
        vm.stopPrank();

        uint256 market1SupplyAmount = 100e18; //user 1 will supply this 
        uint256 market2BorrowAmount = 500e18; //user 1 will borrow this

        vm.startPrank(user1);
        market1.approve(address(ib), market1SupplyAmount);
        ib.supply(user1, user1, address(market1), market1SupplyAmount);

        ib.borrow(user1, user1, address(market2), market2BorrowAmount); 
        vm.stopPrank();
    }

    function test_liquidation_delisting_poc() public {
        /**
         * collateral value = 100 * 0.8 * 1500 = 120,000
         * liquidation collateral value = 100 * 0.9 * 1500 = 135,000
         * borrowed value = 500 * 200 = 100,000
         */
        (uint256 collateralValue, uint256 debtValue) = ib.getAccountLiquidity(user1); 
        console.log("User1 collateralValue: ", collateralValue / 1e18);
        console.log("User1 debtValue: ", debtValue / 1e18);

        bool res1 = ib.isUserLiquidatable(user1);
        console.log("Is liquidatable user 1? ", res1);

        int256 newMarket1Price = 1200e8;
        setPriceToRegistry(registry, admin, address(market1), Denominations.USD, newMarket1Price);

        console.log("Now, delisting the collateral market... ");
        vm.startPrank(admin);
        configurator.softDelistMarket(address(market1));
        configurator.adjustMarketCollateralFactor(address(market1), 0);
        configurator.adjustMarketLiquidationThreshold(address(market1), 0);
        configurator.hardDelistMarket(address(market1));
        vm.stopPrank();

        (collateralValue, debtValue) = ib.getAccountLiquidity(user1);
        console.log("User1 collateralValue: ", collateralValue / 1e18);
        console.log("User1 debtValue: ", debtValue / 1e18);

        bool result = ib.isUserLiquidatable(user1);
        console.log("Is liquidatable user 1? ", result);

        // User2 liquidates user1.
        uint256 repayAmount = 100e18; 
        vm.startPrank(user2);
        market2.approve(address(ib), repayAmount);

        vm.expectRevert("collateral market not listed");
        ib.liquidate(user2, user1, address(market2), address(market1), repayAmount);
        //ib.redeem(user2, user2, address(market1), type(uint256).max); // not possible because liquidation fails
        vm.stopPrank();

    }

}

Tool used

Foundry Manual Review

Recommendation

For markets that are currently configured as collateral, delisting process should be different, the best user-friendly approach I can think of, a grace period should be announced before delisting, where users are informed that the collaterals will not be honored anymore after that period of time, and next upon delisting, if there are still any positions collateralized with that to-be-delisted collateral, they should be forcefully liquidated - for example, by adjusting their liquidation threshold to a point where all positions can be liquidated. A check on hardDelist something like "if its a collateral market, then check if there are any loans taken with this collateral" may be helpful to prevent bad debts.

[ArmedGoose]

Escalate for 10 USDC I believe this is a valid issue, would love you to take a second look on that Already the poc is pretty extensive I believe there is nothing to add. There is no functionality to check if certain loan is collateralized with to-be-delisted collateral. If such exist, then a bad debt may occur.

[sherlock-admin]

Escalate for 10 USDC I believe this is a valid issue, would love you to take a second look on that Already the poc is pretty extensive I believe there is nothing to add. There is no functionality to check if certain loan is collateralized with to-be-delisted collateral. If such exist, then a bad debt may occur.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[0xffff11]

I really appreciate the well explained issue here. I see it as an admin error. They should actually check for this not to happen before delisting. Though I will raise it internally to see more opinions.

[hrishibhat]

Result: Low Unique Considering this issue a low as it can be considered an admin error. since only admin action can lead to such a situation

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• ArmedGoose: rejected