stuart_the_minion - The repaying function calculates interest rates with un-updated total debts values

[sherlock-admin2]

stuart_the_minion

Medium

The repaying function calculates interest rates with un-updated total debts values

Summary

The Repaying function calculates interest rates with out-of-date total debts value

Vulnerability Detail

The interest rates updating step should be done as post-payment process for all pool operations except for flash loaning. (Check this comment)

Updating interest rates as a post-payment process facilitates reserves accruing borrow index with the latest borrow rate till the next pool operation.

And, as can be seen from the code snippet below, the updateInterestRates() function requires updated debt shares to calculate the borrow and interest.

  function updateInterestRates(
    ... ...
  ) internal {
    ... ...
    vars.totalDebt = _cache.nextDebtShares.rayMul(_cache.nextBorrowIndex); // <-- @audit Should use updated debt shares

    (vars.nextLiquidityRate, vars.nextBorrowRate) = IReserveInterestRateStrategy(_reserve.interestRateStrategyAddress)
      .calculateInterestRates(
        _position,
        _data,
        DataTypes.CalculateInterestRatesParams({
          liquidityAdded: _liquidityAdded,
          liquidityTaken: _liquidityTaken,
          totalDebt: vars.totalDebt,
          reserveFactor: _reserveFactor,
          reserve: _reserveAddress
        })
      );
    ... ...
  }

Meanwhile, the repayDebt() function subtracts debt shares from the total and position debt shares in proportion to the repaid assets amount.

  function repayDebt(
    DataTypes.PositionBalance storage self,
    DataTypes.ReserveSupplies storage supply,
    uint256 amount,
    uint128 index
  ) internal returns (uint256 sharesBurnt) {
    sharesBurnt = amount.rayDiv(index);
    require(sharesBurnt != 0, PoolErrorsLib.INVALID_BURN_AMOUNT);
    self.lastDebtLiquidtyIndex = index;
    self.debtShares -= sharesBurnt;
    supply.debtShares -= sharesBurnt;
  }

Therefore, calling the updateInterestRates() function before repayDebt() means that the un-deducted debt shares value is used to calculate the new interest rate, which will make a position to keep higher borrow rate till the next call.

Impact

The updating borrow interest rate with un-deducted debt shares will accrue the borrow index with the higher borrow rate till the next call.

Thus, a borrower or repayer of the next call will have to pay higher assets than expected, and such over-paying will decrease pool users' incentives.

Code Snippet

pool/logic/BorrowLogic.sol#L139-L152

Tool used

Manual Review

Recommendation

The updateInterestRates() function call should go after repayDebt():

  function executeRepay(
    ... ...
  ) external returns (DataTypes.SharesType memory payback) {
    ... ...

    // update balances and total supplies
    payback.shares = balances.repayDebt(totalSupplies, payback.assets, cache.nextBorrowIndex);
    cache.nextDebtShares = totalSupplies.debtShares;

    reserve.updateInterestRates(
      totalSupplies,
      cache,
      params.asset,
      IPool(params.pool).getReserveFactor(),
      payback.assets,
      0,
      params.position,
      params.data.interestRateData
    );

    ... ...
  }

Duplicate of #413