Incorrect Interest Accrual for Expired Withdrawal Batches After Repayment

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarketBase.sol#L406-L431

Vulnerability details

Proof of Concept

The current implementation of interest accrual for withdrawal batches can potentially lead to a situation where an expired withdrawal batch does not receive the full interest it is entitled to when a repayment occurs shortly after the batch expiry. In the normal functioning of the system, interest is accrued and withdrawals are processed as soon as liquidity becomes available, not strictly at the expiry time. So if liquidity is not available, interest should keep on accruing. However, for batches that have just expired, the system assumes that liquidity was available at the expiry time, which can lead to the issue described below.

In repay and repayAndProcessUnpaidWithdrawalBatches, the repayment is processed before updating the state:

  function repay(uint256 amount) external nonReentrant sphereXGuardExternal {
      if (amount == 0) revert_NullRepayAmount();

      asset.safeTransferFrom(msg.sender, address(this), amount);
      emit_DebtRepaid(msg.sender, amount);

      MarketState memory state = _getUpdatedState();
      if (state.isClosed) revert_RepayToClosedMarket();

      // Execute repay hook if enabled
      hooks.onRepay(amount, state, _runtimeConstant(0x24));

      _writeState(state);
    }

In _getUpdatedState, expired withdrawal batches are processed before accruing interest up to the current timestamp:

function _getUpdatedState() internal returns (MarketState memory state) {
      state = _state;
      // Handle expired withdrawal batch
      if (state.hasPendingExpiredBatch()) {
        uint256 expiry = state.pendingWithdrawalExpiry;
        // Only accrue interest if time has passed since last update.
        // This will only be false if withdrawalBatchDuration is 0.
        uint32 lastInterestAccruedTimestamp = state.lastInterestAccruedTimestamp;
        if (expiry != lastInterestAccruedTimestamp) {
          (uint256 baseInterestRay, uint256 delinquencyFeeRay, uint256 protocolFee) = state
            .updateScaleFactorAndFees(
              delinquencyFeeBips,
              delinquencyGracePeriod,
              expiry //only accrues interest till expiry
            );
          emit_InterestAndFeesAccrued(
            lastInterestAccruedTimestamp,
            expiry,
            state.scaleFactor,
            baseInterestRay,
            delinquencyFeeRay,
            protocolFee
          );
        }
        _processExpiredWithdrawalBatch(state);
      }
...  

}

In _processExpiredWithdrawalBatch, the system attempts to process the batch with available liquidity:

function _processExpiredWithdrawalBatch(MarketState memory state) internal {
  uint32 expiry = state.pendingWithdrawalExpiry;
  WithdrawalBatch memory batch = _withdrawalData.batches[expiry];

  if (batch.scaledAmountBurned < batch.scaledTotalAmount) {
    uint256 availableLiquidity = batch.availableLiquidityForPendingBatch(state, totalAssets());
    if (availableLiquidity > 0) {
      _applyWithdrawalBatchPayment(batch, state, expiry, availableLiquidity);
    }
  }
  // ...
}

This sequence of operations can lead to the following scenario:

1. A withdrawal batch expires at time T.
2. A borrower repays at time T+1.
3. The repayment is processed, increasing available liquidity.
4. _getUpdatedState is called.
5. The expired batch is processed with interest only up to time T, using the newly available liquidity.
6. Interest is accrued up to T+1, but this doesn't benefit the already-processed expired batch.

As a result, the expired withdrawal batch loses out on the interest it should have earned between T and T+1 for the portion of the withdrawal that was processed using the new liquidity from the repayment.

Recommended Mitigation Steps

To address this issue accrue interest up to the current block timestamp before processing expired batches.

Assessed type

Other

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid