`WildcatMarketConfig#nukeFromOrbit()` reverts even if the lender has been marked as sanctioned

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarketConfig.sol#L82-L88

Vulnerability details

Impact

The borrower of the market might not be able to force the sanctioned lender into a withdrawal request.

Proof of Concept

When a borrower creates a market hooked by a fixed-term hook, all lenders are prohibited from withdrawing their assets from the market before the fixed-term time has elapsed. If a lender is marked as sanctioned, anyone can call [WildcatMarketConfig#nukeFromOrbit()]() to force it into a withdrawal request. However, the call might revert if fixedTermEndTime has not yet elapsed.

Copy below codes to WildcatMarket.t.sol and run forge test --match-test test_nukeFromOrbit_BeforeFixedTermExpired:

  function test_nukeFromOrbit_BeforeFixedTermExpired() external {
    //@audit-info deploy a FixedTermLoanHooks template
    address fixedTermHookTemplate = LibStoredInitCode.deployInitCode(type(FixedTermLoanHooks).creationCode);
    hooksFactory.addHooksTemplate(
      fixedTermHookTemplate,
      'FixedTermLoanHooks',
      address(0),
      address(0),
      0,
      0
    );

    vm.startPrank(borrower);
    //@audit-info borrower deploy a FixedTermLoanHooks hookInstance
    address hooksInstance = hooksFactory.deployHooksInstance(fixedTermHookTemplate, '');
    DeployMarketInputs memory parameters = DeployMarketInputs({
      asset: address(asset),
      namePrefix: 'name',
      symbolPrefix: 'symbol',
      maxTotalSupply: type(uint128).max,
      annualInterestBips: 1000,
      delinquencyFeeBips: 1000,
      withdrawalBatchDuration: 10000,
      reserveRatioBips: 10000,
      delinquencyGracePeriod: 10000,
      hooks: EmptyHooksConfig.setHooksAddress(address(hooksInstance))
    });
    //@audit-info borrower deploy a market hooked by a FixedTermLoanHooks hookInstance
    address market = hooksFactory.deployMarket(
      parameters,
      abi.encode(block.timestamp + (365 days)),
      bytes32(uint(1)),
      address(0),
      0
    );
    vm.stopPrank();
    //@audit-info lenders can only withdraw their asset one year later
    assertEq(FixedTermLoanHooks(hooksInstance).getHookedMarket(market).fixedTermEndTime, block.timestamp + (365 days));
    //@audit-info alice deposit 50K asset into market
    vm.startPrank(alice);
    asset.approve(market, type(uint).max);
    WildcatMarket(market).depositUpTo(50_000e18);
    vm.stopPrank();
    //@audit-info alice is marked as sanctioned
    MockSanctionsSentinel(WildcatMarket(market).sentinel()).sanction(alice);
    assertTrue(MockSanctionsSentinel(WildcatMarket(market).sentinel()).isSanctioned(borrower, alice));
    //@audit-info however, borrower can not force alice into a withdrawal request due to the unexpired fixed term.
    vm.expectRevert(FixedTermLoanHooks.WithdrawBeforeTermEnd.selector);
    vm.prank(borrower);
    WildcatMarket(market).nukeFromOrbit(alice);
  }

Tools Used

Manual review

Recommended Mitigation Steps

No any restriction should be imposed when WildcatMarketConfig#nukeFromOrbit() is called to force the sanctioned lender into a withdrawal request: https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol

  function _blockAccount(MarketState memory state, address accountAddress) internal override {
    Account memory account = _accounts[accountAddress];
    if (account.scaledBalance > 0) {
      uint104 scaledAmount = account.scaledBalance;

      uint256 normalizedAmount = state.normalizeAmount(scaledAmount);

      uint32 expiry = _queueWithdrawal(
        state,
        account,
        accountAddress,
        scaledAmount,
        normalizedAmount,
+       true,
        msg.data.length
      );

      emit_SanctionedAccountAssetsQueuedForWithdrawal(
        accountAddress,
        expiry,
        scaledAmount,
        normalizedAmount
      );
    }
  }

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

  function _queueWithdrawal(
    MarketState memory state,
    Account memory account,
    address accountAddress,
    uint104 scaledAmount,
    uint normalizedAmount,
+   bool blocked,
    uint baseCalldataSize
  ) internal returns (uint32 expiry) {

    // Cache batch expiry on the stack for gas savings
    expiry = state.pendingWithdrawalExpiry;

    // If there is no pending withdrawal batch, create a new one.
    if (state.pendingWithdrawalExpiry == 0) {
      // If the market is closed, use zero for withdrawal batch duration.
      uint duration = state.isClosed.ternary(0, withdrawalBatchDuration);
      expiry = uint32(block.timestamp + duration);
      emit_WithdrawalBatchCreated(expiry);
      state.pendingWithdrawalExpiry = expiry;
    }

    // Execute queueWithdrawal hook if enabled
-   hooks.onQueueWithdrawal(accountAddress, expiry, scaledAmount, state, baseCalldataSize);
+   if (!blocked) {
+       hooks.onQueueWithdrawal(accountAddress, expiry, scaledAmount, state, baseCalldataSize);
+   }
    // Reduce account's balance and emit transfer event
    account.scaledBalance -= scaledAmount;
    _accounts[accountAddress] = account;

    emit_Transfer(accountAddress, address(this), normalizedAmount);

    WithdrawalBatch memory batch = _withdrawalData.batches[expiry];

    // Add scaled withdrawal amount to account withdrawal status, withdrawal batch and market state.
    _withdrawalData.accountStatuses[expiry][accountAddress].scaledAmount += scaledAmount;
    batch.scaledTotalAmount += scaledAmount;
    state.scaledPendingWithdrawals += scaledAmount;

    emit_WithdrawalQueued(expiry, accountAddress, scaledAmount, normalizedAmount);

    // 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);
    }

    // Update stored batch data
    _withdrawalData.batches[expiry] = batch;

    // Update stored state
    _writeState(state);
  }

  /**
   * @dev Create a withdrawal request for a lender.
   */
  function queueWithdrawal(
    uint256 amount
  ) external nonReentrant sphereXGuardExternal returns (uint32 expiry) {
    MarketState memory state = _getUpdatedState();

    uint104 scaledAmount = state.scaleAmount(amount).toUint104();
    if (scaledAmount == 0) revert_NullBurnAmount();

    // Cache account data
    Account memory account = _getAccount(msg.sender);

    return
-     _queueWithdrawal(state, account, msg.sender, scaledAmount, amount, _runtimeConstant(0x24));
+     _queueWithdrawal(state, account, msg.sender, scaledAmount, amount, false, _runtimeConstant(0x24));
  }

  /**
   * @dev Queue a withdrawal for all of the caller's balance.
   */
  function queueFullWithdrawal()
    external
    nonReentrant
    sphereXGuardExternal
    returns (uint32 expiry)
  {
    MarketState memory state = _getUpdatedState();

    // Cache account data
    Account memory account = _getAccount(msg.sender);

    uint104 scaledAmount = account.scaledBalance;
    if (scaledAmount == 0) revert_NullBurnAmount();

    uint256 normalizedAmount = state.normalizeAmount(scaledAmount);

    return
      _queueWithdrawal(
        state,
        account,
        msg.sender,
        scaledAmount,
        normalizedAmount,
+       false,
        _runtimeConstant(0x04)
      );
  }

Assessed type

Context

[d1ll0n]

This is documented in the Known Issues page.

These withdrawals are not given special treatment because we don't want sanctioned accounts to receive withdrawal priority over non-sanctioned accounts.

[3docSec]

Known issue as per sponsor's comment

[c4-judge]

3docSec marked the issue as unsatisfactory: Out of scope