Delinquency fee can be totally or partially avoided due to MarketState.isDelinquent only being updated at the end of a market operation

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarketBase.sol#L406 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarketWithdrawals.sol#L283 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L202

Vulnerability details

Impact

MarketState.isDelinquent is only updated at the end of a market operation. When a non-static call made to the market happens, if the last state update was not delinquent, the fees and scale factor will be computed as though the market is not delinquent, even if at that moment the market's required liquidity is greater than the total assets and the delinquency grace period has already passed.

The market computes fees incrementally on non-static call made to it. If the call that changes the state from non delinquent to delinquent happens after the delinquencyGracePeriod, then no delinquencyFee will be charged for the period between when the grace period ended and when the call was made. In the worse case, if that call happens to be a repayment which increases the totalAssets above the required liquidity, then the market would never enter in delinquency and no delinquency fees will be charged to the borrower.

Proof of Concept

Scenario 1: Avoidance of delinquency fee payment:

1. Lender deposits funds into a market
2. Borrower borrow's the maximum borrowable amount
3. The next second, the required liquidity will be greater than the total assets in the market
4. Time passes beyond the delinquencyGracePeriod (for example: delinquencyGracePeriod + 3 days go by)
5. The borrower makes a repayment to leave the total assets above the required liquidity

The payment that the borrower makes in this scenario does not include delinquency fees (the new scale factor was computed with delinquencyFee = 0). The market never enters in delinquency, and the delinquency cooldown period (3 days in this case) never enters into play.

Scenario 2: Partial avoidance of delinquency fee payment:

1. Steps 1 to 4 of the previous example are the same
2. Make a non static call to the contract, for example, the lender might make a deposit

In this scenario, the borrower will not pay delinquency fees for the first 3 days after the grace period where the total assets where below the required liquidity

The following test shows a proof of concept of the first scenario

pragma solidity >=0.8.20;

import './BaseMarketTest.sol';
import 'src/libraries/MathUtils.sol';
import 'src/libraries/MarketState.sol';

contract AuditMarketTest is BaseMarketTest {
  using MathUtils for uint256;

  function test_audit_delinquencyFeeAvoided() external {
    vm.prank(alice);
    market.depositUpTo(50_000e18);

    // borrow the maximum amount possible to force delinquency in the next block
    uint256 borrowTimestamp = block.timestamp;
    uint256 borrowedAmount = market.borrowableAssets();

    vm.prank(borrower);
    market.borrow(borrowedAmount);
    MarketState memory initialState = market.currentState();

    // 1 second into the future the market is notionally delinquent,
    // although the state does not reflect this yet because no write operations happened.
    // this is known and expected.
    vm.warp(borrowTimestamp + 1);
    assertGt(market.currentState().liquidityRequired(), market.totalAssets());

    // go enough time into the future to trigger delinquency fee
    uint256 delinquencyGracePeriod = market.delinquencyGracePeriod();
    uint256 secondsIntoTheFuture = delinquencyGracePeriod + 3 days;
    vm.warp(borrowTimestamp + secondsIntoTheFuture); // update block.timestamp
    vm.roll(block.number + secondsIntoTheFuture / 20); // update block.number

    // if the borrower were to pay back the loan right now, no penalty fees
    //    will be charged, although 3 days have passed since the delinquency
    //    grace period ended

    // compute the liquidity required to get the market to non delinquent state.
    // this shows that delinquency fees are 0 (not taken into account in the computation)
    uint256 baseInterestRay = uint256(initialState.annualInterestBips).bipToRay() * secondsIntoTheFuture / 365 days;

    uint256 protocolFee = uint256(initialState.scaledTotalSupply)
      .rayMul(uint256(initialState.scaleFactor)
        .rayMul(uint(initialState.protocolFeeBips).bipMul(baseInterestRay)));

    uint256 scaleFactorWithoutPenalty = initialState.scaleFactor + uint256(initialState.scaleFactor).rayMul(baseInterestRay);

    uint256 scaledRequiredReserves = uint256(initialState.scaledTotalSupply)
      .bipMul(initialState.reserveRatioBips)
      .rayMul(scaleFactorWithoutPenalty);

    uint256 liquidityRequiredWithoutPenalty = scaledRequiredReserves + protocolFee;

    // compare the computed value with the actual value needed by the market
    // to avoid delinquency
    MarketState memory newState = market.currentState();

    // market is not delinquent, scale factor nor liquidityRequired()
    //    took into account the 3 days of delinquency fees
    assertFalse(newState.isDelinquent);
    assertEq(newState.scaleFactor, scaleFactorWithoutPenalty);
    assertEq(newState.liquidityRequired(), liquidityRequiredWithoutPenalty);

    // the borrower can repay and avoid delinquency completely
    uint256 repayAmount = newState.liquidityRequired() - market.totalAssets();
    vm.startPrank(borrower);
    asset.approve(address(market), repayAmount);
    market.repay(repayAmount);
    vm.stopPrank();

    // the market has never been delinquent, and the delinquency was avoided
    // in the state update
    MarketState memory finalState = market.currentState();
    assertFalse(finalState.isDelinquent);
    assertEq(finalState.liquidityRequired(), market.totalAssets());
  }
}

Tools Used

Test suite

Recommended Mitigation Steps

The mitigation might require to make several changes in the contracts.

It is recommended to check if the market is currently in a delinquent state before updating the fees and scale factor in the MarketBase::_getUpdatedState function.

One way to do that could be with a change like the following in MarketBase::_getUpdatedState:

  function _getUpdatedState() internal returns (MarketState memory state) {
    state = _state;
    (MarketState memory currentState,,) = _calculateCurrentState();
    state.isDelinquent = currentState.liquidityRequired() > totalAssets();

    // rest of the function here

Although, given the amount of optimizations the project has, the development team will probably want to implement this in a more efficient way.

Also, special attention should be payed to the following functions:

• WildcatMarket::repay()
• WildcatMarketWithrawals::repayAndProcessUnpaidWithdrawalBatches

Those make global state changes before calling the _getUpdatedState() function. Those functions transfers funds from the borrower to the market before getting the updated state, which could result in a delinquency state not being triggered with this solution. In such cases it would be convenient to at least update the delinquency state (if updating the whole market state is not an option) before making any state changing operation like transferring funds.

Assessed type

Math

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid