No State Update After Transfer in repayOutstandingDebt and repayDelinquentDebt Leads to Inaccurate Interest Accrual

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarket.sol#L180-L183 https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarket.sol#L186-L190

Vulnerability details

The repayOutstandingDebt and repayDelinquentDebt functions in the WildcatMarket contract update the market state only once before processing the repayment, leading to a situation where the repaid amount is not immediately credited against pending withdrawals. This can result in excessive interest accrual and inaccurate debt calculations.

Proof of Concept

Here's the relevant code from WildcatMarket.sol:

function repayOutstandingDebt() external nonReentrant sphereXGuardExternal {
  MarketState memory state = _getUpdatedState();
  uint256 outstandingDebt = state.totalDebts().satSub(totalAssets());
  _repay(state, outstandingDebt, 0x04);
  _writeState(state);
}

function repayDelinquentDebt() external nonReentrant sphereXGuardExternal {
  MarketState memory state = _getUpdatedState();
  uint256 delinquentDebt = state.liquidityRequired().satSub(totalAssets());
  _repay(state, delinquentDebt, 0x04);
  _writeState(state);
}

The issue arises because:

1. The state is updated at the beginning of the function call.
2. The repayment amount is calculated based on this updated state.
3. The repayment is processed, transferring assets to the contract.
4. The state is written back to storage without reflecting the repayment.

Importantly, this behavior differs from other repayment methods in the contract. For example, the repay function updates the state after the asset transfer:

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();
  // ... rest of the function
}

This difference in behavior can lead to inconsistencies in how repayments are processed and reflected in the contract state. Furthermore, in the _getUpdatedState function, the available liquidity calculated using totalAssets() is used to mark pending withdrawals:

solidity
function _getUpdatedState() internal returns (MarketState memory state) {
  // ... other code ...

  if (state.pendingWithdrawalExpiry != 0) {
    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);
        _withdrawalData.batches[expiry] = batch;
      }
    }
  }

  // ... rest of the function
}

In the case of repayOutstandingDebt and repayDelinquentDebt, because the state update occurs before the repayment, this calculation of available liquidity does not include the repaid amount. This means that pending withdrawals are not immediately credited with the newly available liquidity from the repayment and they keep accruing interest.

This results in borrower paying additional interest beyond what should have been the case.

Recommended Mitigation Steps

Update the state again post transfer.

Assessed type

Other

[c4-judge]

3docSec marked the issue as satisfactory