JCN - Interest rates are overestimated after every repayment

[sherlock-admin2]

JCN

Medium

Interest rates are overestimated after every repayment

Summary

Interest rates are overestimated after every repayment

Vulnerability Detail

The cach.nextDebtShares value is used to calculate the updated totalDebt of the reserve and then calculate the new interest rates based on this updated value. However, during repayments the cache.nextDebtShares is updated after interest rates are updated, as a result the cache.nextDebtShares reflects the previous totalDebt before repayment:

BorrowLogic::executeRepay

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


    // update balances and total supplies
    payback.shares = balances.repayDebt(totalSupplies, payback.assets, cache.nextBorrowIndex);
    cache.nextDebtShares = totalSupplies.debtShares; // @audit: should have been updated before updating interest rates

ReserveLogic::updateInterestRates

    vars.totalDebt = _cache.nextDebtShares.rayMul(_cache.nextBorrowIndex);


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

This leads to an overestimation of interest rates after every repayment, since a reduced totalDebt should translate to a lower utilization and thus a lower interest rate. These overestimated interest rates will persist until a state changing call occurs for the affected reserve.

Proof of Concept

Modify the following lines in PoolRepayTests.t.sol and run test with forge test --mc PoolRepayTests --mt testRepayFullBorrow -vvv:

diff --git a/test/forge/core/pool/PoolRepayTests.t.sol b/test/forge/core/pool/PoolRepayTests.t.sol
index 63ea734..db7464f 100644
--- a/test/forge/core/pool/PoolRepayTests.t.sol
+++ b/test/forge/core/pool/PoolRepayTests.t.sol
@@ -48,17 +48,37 @@ contract PoolRepayTests is PoolSetup {

     tokenA.approve(address(pool), UINT256_MAX);

+    emit log_named_uint("borrow rate before repayment", pool.getReserveData(address(tokenA)).borrowRate);
+    emit log_named_uint("liquidity rate before repayment", pool.getReserveData(address(tokenA)).liquidityRate);
+
     pool.repaySimple(address(tokenA), UINT256_MAX, 0);
     uint256 tokenBalanceAfter = tokenA.balanceOf(alice);

     vm.stopPrank();

+    uint256 totalDebtInReserve = pool.totalDebt(address(tokenA));
+    assertEq(totalDebtInReserve, 0); // reserve has no debt so interest rates should be 0 now
+
+    emit log_named_uint("total debt in reserve after repayment", totalDebtInReserve);
+
+    emit log_named_uint("borrow rate after repayment", pool.getReserveData(address(tokenA)).borrowRate);
+    emit log_named_uint("liquidity rate after repayment", pool.getReserveData(address(tokenA)).liquidityRate);
+
     uint256 debtBalanceAfter = pool.getDebt(address(tokenA), alice, 0);
     assertEq(debtBalanceAfter, 0);

     // greater than the actual borrow amount as interest accrued
     assertGt(tokenBalanceBefore - tokenBalanceAfter, borrowAmount);
     assertEq(pool.getUserConfiguration(alice, 0).isBorrowing(pool.getReserveData(address(tokenA)).id), false);
+
+    // interest rates normalized with state changing operation on reserve
+    address user = address(0x69420);
+    _mintAndApprove(user, tokenA, 1, address(pool));
+    vm.prank(user);
+    pool.supplySimple(address(tokenA), user, 1, 0); // supply negligible amount to observe expected interest rates after repayment
+
+    emit log_named_uint("expected borrow rate after repayment", pool.getReserveData(address(tokenA)).borrowRate);
+    emit log_named_uint("expected liquidity rate after repayment", pool.getReserveData(address(tokenA)).liquidityRate);
   }

   function testRepayPartialBorrow() external {

Output from logs:

[PASS] testRepayFullBorrow() (gas: 588630)
Logs:
  borrow rate before repayment: 59574468085106382978723404
  liquidity rate before repayment: 23829787234042553191489362
  total debt in reserve after repayment: 0
  borrow rate after repayment: 42582954161102301485009517
  liquidity rate after repayment: 12175067899866767314611881
  expected borrow rate after repayment: 0
  expected liquidity rate after repayment: 0

Impact

Interest rates will be overestimated after every repayment. This overestimation is more severe the smaller the difference between the debt repaid and the totalDebt before repayment. The interest rates will only be normalized when a state changing action occurs for the affected reserve (i.e. supply, withdraw, borrow, repay). Until that action occurs, suppliers will be earning inflated interest while borrowers will be paying more interest than they are supposed to.

Note that the POC above outlines a specific scenario in which the ultimate impact of this bug will manifest:
When a repayment results in all debt in the reserve being repaid, the interest rates should become 0, but as shown above, they will only decrease due to cash entering the reserve. This means that any suppliers in the reserve after this point (and the protocol) will be earning non-existent interest as there are no more borrowers in the reserve to pay the interest (totalDebt == 0). Therefore, when suppliers start withdrawing from the pool, they will be withdrawing excess funds (due to fake interest), which ultimately means they are unknowingly stealing from other suppliers' funds. In this state, the last supplier to withdraw will lose funds.

Code Snippet

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/BorrowLogic.sol#L139-L152

Tool used

Manual Review

Recommendation

Update interest rates after repaying debt and updating the cach.nextDebtShares.

Duplicate of #413