sherlock-audit / 2023-05-ironbank-judging

2 stars 2 forks source link

devScrooge - Borrower can not be seized even if a liquidation occurs #410

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

devScrooge

high

Borrower can not be seized even if a liquidation occurs

Summary

If a market is initialized with liquidationThreshold, collateralFactor as 0, the liquidationBonus can also be 0. If the configuration of the market is later changed setting values different from 0 to liquidationThreshold, collateralFactor but not liquidationBonus, the seize amount will be 0. This will result on the borrower not being seize any amount of tokens and the liquidator receving a total amount of 0 due to the liquidation.

Vulnerability Detail

The MarketConfigurator.sol contract implements a function used to set the value for liquidationBonus to a market:

function adjustMarketLiquidationBonus(address market, uint16 liquidationBonus) external onlyOwner {
        DataTypes.MarketConfig memory config = getMarketConfiguration(market);
        require(config.isListed, "not listed");
        if (liquidationBonus > 0) {
            require(
                liquidationBonus > MIN_LIQUIDATION_BONUS && liquidationBonus <= MAX_LIQUIDATION_BONUS,
                "invalid liquidation bonus"
            );
            require(
                uint256(config.liquidationThreshold) * uint256(liquidationBonus) / FACTOR_SCALE
                    <= MAX_LIQUIDATION_THRESHOLD_X_BONUS,
                "liquidation threshold * liquidation bonus larger than 100%"
            );
        } else {
            require(
                config.collateralFactor == 0 && config.liquidationThreshold == 0,
                "collateral factor or liquidation threshold not zero"
            );
        }

        config.liquidationBonus = liquidationBonus;
        ironBank.setMarketConfiguration(market, config);

        emit MarketLiquidationBonusSet(market, liquidationBonus);
    }

This function only allows liquidationBonus to be set as 0 if collateralFactor and liquidationThreshold, which makes sense:

require(
    config.collateralFactor == 0 && config.liquidationThreshold == 0,
    "collateral factor or liquidation threshold not zero"
);

The problem is that the following can happen:

  1. collateralFactor and liquidationThreshold are both 0.
  2. liquidationBonus is correctly set to 0 since it meets the 1. point requirements.
  3. liquidationThreshold is set to 5 (for example).
  4. collateralFacctor is set to 3.
  5. liquidationBonus remains as 0.

This situation breaks the invariant presented on the adjustMarketLiquidationBonus of liquidationBonus only being 0 if collateralFactor and `liquidationThreshold are also 0.

The describe situation can take place due the fact that the adjustMarketLiquidationThreshold and adjustMarketCollateralFactor functions do not check that liquidationBonus is not 0 when trying to set liquidationThreshold and collateralFactor to any value different from 0.

Impact

The liquidationBonus variable is used to calculate the total seize amount when a liquidation takes place. It makes sense that it can be set to 0 if the collateralFactor and liquidationThreshold are also set to 0 because this means that borrows and liquidation can not happen but in the described above scenario, a user is able to borrow and not being seized when a liquidation takes place. This is because liquidationBonus is used to calculate the amount to be seized, it is done in the _getLiquidationSeizeAmount on the `IronBank.sol contract:

function _getLiquidationSeizeAmount(
        address marketBorrow,
        address marketCollateral,
        DataTypes.Market storage mCollateral,
        uint256 repayAmount
    ) internal view returns (uint256) {
        uint256 borrowMarketPrice = PriceOracleInterface(priceOracle).getPrice(marketBorrow);
        uint256 collateralMarketPrice = PriceOracleInterface(priceOracle).getPrice(marketCollateral);
        require(borrowMarketPrice > 0 && collateralMarketPrice > 0, "invalid price");

        // collateral amount = repayAmount * liquidationBonus * borrowMarketPrice / collateralMarketPrice
        // IBToken amount = collateral amount / exchangeRate
        //   = repayAmount * (liquidationBonus * borrowMarketPrice) / (collateralMarketPrice * exchangeRate)
        uint256 numerator = (mCollateral.config.liquidationBonus * borrowMarketPrice) / FACTOR_SCALE;
        uint256 denominator = (_getExchangeRate(mCollateral) * collateralMarketPrice) / 1e18;

        return (repayAmount * numerator) / denominator;
    }

The _getLiquidationSeizeAmount function is called on the liquidate function to calculate the ibTokenAmount variable which is the total amount of tokens that needs to be seized to the borrower:

function liquidate(
        address liquidator,
        address borrower,
        address marketBorrow,
        address marketCollateral,
        uint256 repayAmount
    ) external nonReentrant isAuthorized(liquidator) {
        DataTypes.Market storage mBorrow = markets[marketBorrow];
        DataTypes.Market storage mCollateral = markets[marketCollateral];
        require(mBorrow.config.isListed, "borrow market not listed");
        require(mCollateral.config.isListed, "collateral market not listed");
        require(isMarketSeizable(mCollateral), "collateral market cannot be seized");
        require(!isCreditAccount(borrower), "cannot liquidate credit account");
        require(liquidator != borrower, "cannot self liquidate");

        _accrueInterest(marketBorrow, mBorrow);
        _accrueInterest(marketCollateral, mCollateral);

        // Check if the borrower is actually liquidatable.
        require(_isLiquidatable(borrower), "borrower not liquidatable");

        // Repay the debt.
        repayAmount = _repay(mBorrow, liquidator, borrower, marketBorrow, repayAmount);

        // Seize the collateral.
        uint256 ibTokenAmount = _getLiquidationSeizeAmount(marketBorrow, marketCollateral, mCollateral, repayAmount);
        _transferIBToken(marketCollateral, mCollateral, borrower, liquidator, ibTokenAmount);
        IBTokenInterface(mCollateral.config.ibTokenAddress).seize(borrower, liquidator, ibTokenAmount); // Only emits Transfer event.

        emit Liquidate(liquidator, borrower, marketBorrow, marketCollateral, repayAmount, ibTokenAmount);
    }

Therefore, the borrower will not be seized even if a liquidation takes place.

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/MarketConfigurator.sol#L165-L179 https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/MarketConfigurator.sol#L205-L226 https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/MarketConfigurator.sol#L233-L257 https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/IronBank.sol#L821-L838 https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/IronBank.sol#L505

Tool used

Manual Review

Recommendation

Implements a require statment that check that m.liquidationBonus != 0 when trying to set a value different from 0 to collateralFactor and liquidationThreshold.