FIFO queue is not strict enough

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarketBase.sol#L460

Vulnerability details

Impact

Current implementation slightly favours current expiry batch over unpaid old ones.

Proof of Concept

Within _getUpdateState, there's a check if there's an outstanding expiry batch and if there is, an attempt to fulfil a part of is made.

    if (state.pendingWithdrawalExpiry != 0) {
      uint32 expiry = state.pendingWithdrawalExpiry;
      WithdrawalBatch memory batch = _withdrawalData.batches[expiry];
      if (batch.scaledAmountBurned < batch.scaledTotalAmount) {
        // Burn as much of the withdrawal batch as possible with available liquidity.
        uint256 availableLiquidity = batch.availableLiquidityForPendingBatch(state, totalAssets());
        if (availableLiquidity > 0) {
          _applyWithdrawalBatchPayment(batch, state, expiry, availableLiquidity);
          _withdrawalData.batches[expiry] = batch;
        }
      }
    }

In order to value the FIFO order, it does only check for available liquidity, which basically takes into account previous unpaid batches.

Consider however the following scenario:

1. There's a previous fully unpaid batch for 1% of the funds within the contract.
2. A new withdraw request is made also for 1% of the funds, creating a new expiry batch.
3. Update state is invoked. Currently there are 2% of the funds within the contract (assume protocol fees have just been withdrawn). As there's enough funds to cover for the previous unpaid batch and the protocol fees, a payment is made to the new batch, basically filling it.
4. The newer batch has been filled before the old one. Now there's needed intervention from the borrower in order to fill the old unpaid batch, despite there being enough funds in the contract for the order. Actions from borrower's side may often take long as they're likely to be DAOs/ multi-sigs. DAO proposals often take days before being able to be executed.
5. Time goes by and 1% in fees are accrued. Wildcat protocol takes their fees (as they're prioritized over unpaid withdraw batches).
6. In the end, the funds are distributed and the newer batch has been filled while the old one is not.

Tools Used

Manual review

Recommended Mitigation Steps

Within _getUpdateState check if there's unpaid batches and if there's available liquidity, pay them.

Assessed type

Context

[d1ll0n]

The withdrawal prioritization is listed in the core behavior page of the docs on the repo:

Your suggested mitigation would involve potentially massive increases in gas costs to all lenders interacting with the market when unpaid batches exist, and could even lead to a DOS vector in markets with infrequent use and low batch durations.

[3docSec]

Closing as intended behavior.

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid