sherlock-audit / 2024-04-interest-rate-model-judging

9 stars 5 forks source link

Sentinels - Liquidators can't liquidate borrower if he hasn't entered the `repayMarket` #131

Closed sherlock-admin3 closed 6 months ago

sherlock-admin3 commented 7 months ago

Sentinels

medium

Liquidators can't liquidate borrower if he hasn't entered the repayMarket

Summary

A liquidator can choose a repayMarket - This is where the liquidator is providing funds to cover the borrower's debts. A liquidator can choose this market based on where they can most effectively repay debt.

As a part of the liquidate function we also have a seizeMarket - This market is where the liquidator seizes collateral from the borrower to compensate for the debts they've repaid.

The problem is, that if the borrower hasn't entered the repayMarket he can't be liquidated making liquidations extremely restrictive while the repay market should be the liquidator's choice and the liquidator should have the choice from which market he decides to repay.

Vulnerability Detail

Let's take a look at the first lines of the liquidate function:

  function liquidate(
    address borrower,
    uint256 maxAssets,
    Market seizeMarket
  ) external whenNotPaused returns (uint256 repaidAssets) {
    //self-liquidation not allowed 
    if (msg.sender == borrower) revert SelfLiquidation();

    maxAssets = auditor.checkLiquidation(this, seizeMarket, borrower, maxAssets);

We call checkLiquidation:

function checkLiquidation(
    Market repayMarket,
    Market seizeMarket,
    address borrower,
    uint256 maxLiquidatorAssets
  ) external view returns (uint256 maxRepayAssets) {
    // if markets are listed, they have the same auditor
    if (!markets[repayMarket].isListed || !markets[seizeMarket].isListed) revert MarketNotListed();

    MarketVars memory repay;
    LiquidityVars memory base;
    uint256 marketMap = accountMarkets[borrower];
    for (uint256 i = 0; marketMap != 0; marketMap >>= 1) {
      if (marketMap & 1 != 0) {
        Market market = marketList[i];
        MarketData storage marketData = markets[market];
        MarketVars memory m = MarketVars({
          price: assetPrice(marketData.priceFeed),
          adjustFactor: marketData.adjustFactor,
          baseUnit: 10 ** marketData.decimals
        });

        if (market == repayMarket) repay = m; <---

        (uint256 collateral, uint256 debt) = market.accountSnapshot(borrower);

        uint256 value = debt.mulDivUp(m.price, m.baseUnit);
        base.totalDebt += value;
        base.adjustedDebt += value.divWadUp(m.adjustFactor);

        value = collateral.mulDivDown(m.price, m.baseUnit);
        base.totalCollateral += value;
        base.adjustedCollateral += value.mulWadDown(m.adjustFactor);
        if (market == seizeMarket) base.seizeAvailable = value;
      }
      unchecked {
        ++i;
      }
    }

    if (base.adjustedCollateral >= base.adjustedDebt) revert InsufficientShortfall();

    LiquidationIncentive memory memIncentive = liquidationIncentive;
    uint256 adjustFactor = base.adjustedCollateral.mulWadDown(base.totalDebt).divWadUp(
      base.adjustedDebt.mulWadUp(base.totalCollateral)
    );
    uint256 closeFactor = (TARGET_HEALTH - base.adjustedCollateral.divWadUp(base.adjustedDebt)).divWadUp(
      TARGET_HEALTH - adjustFactor.mulWadDown(1e18 + memIncentive.liquidator + memIncentive.lenders)
    );
    maxRepayAssets = Math.min(
      Math
        .min(
          base.totalDebt.mulWadUp(Math.min(1e18, closeFactor)),
          base.seizeAvailable.divWadUp(1e18 + memIncentive.liquidator + memIncentive.lenders)
        )
        .mulDivUp(repay.baseUnit, repay.price), <---
      maxLiquidatorAssets < ASSETS_THRESHOLD
        ? maxLiquidatorAssets.divWadDown(1e18 + memIncentive.lenders)
        : maxLiquidatorAssets
    );
  }

The function iterates over the markets entered by the borrower and attempts to initialize a MarketVars structure for the repay market. If the borrower has never interacted with the chosen repay market, the MarketVars structure for the repay market remains uninitialized.

This will lead to a revert in the following line:

 .mulDivUp(repay.baseUnit, repay.price), <---

which will try to divide by 0 as repay.price will return 0 when it's not initialized.

Impact

This issue prevents the function from executing successfully when the repayMarket is not one of the markets the borrower has previously interacted with. It restricts the flexibility of liquidation processes and leads to failures in handling liquidations effectively.

Code Snippet

https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/main/protocol/contracts/Auditor.sol#L217

Tool used

Manual Review

Recommendation

Get the MarketVars parameters directly from the passed repayMarket outside of the entered markets loop.

santipu03 commented 6 months ago

This issue is invalid because when a user borrows from a market, the contract automatically enters the user into that market, and is not possible to exit the market until all debt is repaid.