After lender is unsanctioned, he will not be able to withdraw and deposit again

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L88-L102 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L197-L213

Vulnerability details

Impact

• After lender is unsanctioned, he will not be able to withdraw and deposit again.
• Stuck funds in the Market.

Explanation

If a lender's address is flagged as sanctioned by the Chainanalysis oracle, there are two possible scenarios:

1. Using the nukeFromOrbit() Function: In this case, anyone can invoke the WildcatMarketConfig.sol#nukeFromOrbit() function with the sanctioned user's accountAddress. This function checks if the lender is indeed sanctioned by verifying their status with the Chainanalysis oracle. If confirmed as sanctioned, the lender's account is blocked within the market, and their state is updated accordingly.

   function nukeFromOrbit(address accountAddress) external nonReentrant {
   if (!IWildcatSanctionsSentinel(sentinel).isSanctioned(borrower, accountAddress)) {
   revert BadLaunchCode();
   }
   MarketState memory state = _getUpdatedState();
   _blockAccount(state, accountAddress);
   _writeState(state);
   }

2. Executing executeWithdrawal() as a Sanctioned Lender: In this scenario, the sanctioned lender, while still flagged as sanctioned by the Chainalysis oracle, initiates a withdrawal by calling WildcatMarketWithdrawals.sol#executeWithdrawal(). If the withdrawal is executed while the lender is sanctioned, an escrow contract is created between the lender and the borrower within the market. The lender's market tokens (scaledBalance) are transferred to this escrow contract instead of directly to the lender, and the lender's account is marked as Blocked within the market.

   
   function executeWithdrawal(
   address accountAddress,
   uint32 expiry
   ) external nonReentrant returns (uint256) {
   if (expiry > block.timestamp) {
   revert WithdrawalBatchNotExpired();
   }
   MarketState memory state = _getUpdatedState();

WithdrawalBatch memory batch = _withdrawalData.batches[expiry];
AccountWithdrawalStatus storage status = _withdrawalData.accountStatuses[expiry][
  accountAddress
];

// ...code...

if (IWildcatSanctionsSentinel(sentinel).isSanctioned(borrower, accountAddress)) {
  _blockAccount(state, accountAddress);
  address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
    accountAddress,
    borrower,
    address(asset)
  );
  asset.safeTransfer(escrow, normalizedAmountWithdrawn);
  emit SanctionedAccountWithdrawalSentToEscrow(
    accountAddress,
    escrow,
    expiry,
    normalizedAmountWithdrawn
  );
} else {
  asset.safeTransfer(accountAddress, normalizedAmountWithdrawn);
}

// ...code...

In both situations, the lender's assets are held in escrow, and their market account is restricted with a `Blocked` status.

After same amount of time the lender can become unsanctioned if one of these two things happens:
> 1. The lender is no longer flagged as sanctioned by the Chainalysis oracle.
> 2. The borrower for the market calls `overrideSanction()` for the lender.

> If this happens, then anyone can call `releaseEscrow` in the escrow contract(s) itself, and any assets shifted as part of an `executeWithdrawal()` or `nukeFromOrbit()` call are returned to the lender.
> Subsequently, anyone can return the lender to good grace within their market by calling [`stunningReversal()`](https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L88C1-L102) - which will revert if the lender is still sanctioned unless the borrower overrode the sanction.
```jsx
  function stunningReversal(address accountAddress) external nonReentrant {
    Account memory account = _accounts[accountAddress];
    if (account.approval != AuthRole.Blocked) {
      revert AccountNotBlocked();
    }

    if (IWildcatSanctionsSentinel(sentinel).isSanctioned(borrower, accountAddress)) {
      revert NotReversedOrStunning();
    }

    account.approval = AuthRole.Null; //@note
    emit AuthorizationStatusUpdated(accountAddress, account.approval);

    _accounts[accountAddress] = account;
  }

Proof of Concept

The problem arise that when the lender is sanctioned, he become with approval = AuthRole.Blocked, but when the lender is unsanctioned and return the lender to good grace within their market by calling stunningReversal(), his approval in account become AuthRole.Null. So, after lender is unsanctioned, he will not be able to withdraw and deposit again, because when:

The lender try to deposit or withdraw...

    // Depositing

  function depositUpTo(
    uint256 amount
  ) public virtual nonReentrant returns (uint256 /* actualAmount */) {
    // ...code...

    // Cache account data and revert if not authorized to deposit.
    Account memory account = _getAccountWithRole(msg.sender, AuthRole.DepositAndWithdraw);
    account.scaledBalance += scaledAmount;
    _accounts[msg.sender] = account;

    emit Transfer(address(0), msg.sender, amount);
    emit Deposit(msg.sender, amount, scaledAmount);

    // ...code...

    return amount;
  }

    // Withdrawing

    function queueWithdrawal(uint256 amount) external nonReentrant {
        MarketState memory state = _getUpdatedState();

        // Cache account data and revert if not authorized to withdraw.
        Account memory account = _getAccountWithRole(msg.sender, AuthRole.WithdrawOnly);

        // ..code...

        // Update stored state
        _writeState(state);
  }

... the _getAccountWithRole() function is invoked, which contains the following check:

    if (uint256(account.approval) < uint256(requiredRole)) {
      revert NotApprovedLender();
    }

This check always will revert since account.approval = AuthRole.Null

Tools Used

• Manual Inspection

Mitigation Steps

Update the approval (AuthRole) of account properly in stunningReversal() function

Assessed type

Context

[c4-pre-sort]

minhquanym marked the issue as low quality report

[minhquanym]

Seems invalid. When his approval in account become AuthRole.Null, he can always call updateLenderAuthorization() to get AuthRole.WithdrawOnly

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Invalid

[radeveth]

Hi, @laurenceday!

I just want to note you that this issue can arise after fixing of the #236. (restriction of updateLenderAuthorization() function)