The Wildcat Protocol - serial-coder's results

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L142

Vulnerability details

The WildcatMarket::closeMarket() cannot be executed by a borrower because the WildcatMarketController contract does not implement any function calling it.

Proof of Concept

The closeMarket() is supposed to be called by a borrower to close their market, set the market's annualInterestBips (APR) to 0%, disallow lenders from depositing asset tokens to the market, and transfer the remaining debts to the market for all lenders to retrieve.

A borrower must execute the closeMarket() through the WildcatMarketController contract, as the function is attached to the onlyController modifier. However, I discovered that the WildcatMarketController contract does not implement any function calling the closeMarket().

Therefore, a borrower cannot execute the closeMarket() to close their market.

@>  function closeMarket() external onlyController nonReentrant {
      MarketState memory state = _getUpdatedState();
      state.annualInterestBips = 0;
      state.isClosed = true;
      state.reserveRatioBips = 0;
      if (_withdrawalData.unpaidBatches.length() > 0) {
        revert CloseMarketWithUnpaidWithdrawals();
      }
      uint256 currentlyHeld = totalAssets();
      uint256 totalDebts = state.totalDebts();
      if (currentlyHeld < totalDebts) {
        // Transfer remaining debts from borrower
        asset.safeTransferFrom(borrower, address(this), totalDebts - currentlyHeld);
      } else if (currentlyHeld > totalDebts) {
        // Transfer excess assets to borrower
        asset.safeTransfer(borrower, currentlyHeld - totalDebts);
      }
      _writeState(state);
      emit MarketClosed(block.timestamp);
    }

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarket.sol#L142

Impact

A borrower will be unable to execute the closeMarket() to close their market. Consequently, the borrower cannot disallow lenders from depositing asset tokens (via the depositUpTo() and deposit()) to the market, and the borrower will be forced to pay the lending interest.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a function (in the WildcatMarketController contract) for handling the execution of the WildcatMarket::closeMarket().

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L182 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L188 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L121 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketToken.sol#L36 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L77 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L137

Vulnerability details

The current implementation of the WildcatMarketController::updateLenderAuthorization() leads to the privilege escalation vulnerability, enabling an unauthorized account with AuthRole.Null approval status to be able to escalate its privilege to AuthRole.WithdrawOnly.

Impact

When combining the privilege escalation vulnerability with the front-running and Sybil attacks, a sanctioned lender can evacuate and exit their underlying assets from a market without a permit.

Moreover, the market's borrower or Wildcat protocol owner cannot block the (sanctioned) lender from exiting their asset tokens.

Proof of Concept

This PoC section is divided into two sub-sections:

• Explaining how an unauthorized account can escalate its privilege
• Step-by-step asset evacuation and exiting process

Explaining how an unauthorized account can escalate its privilege

The WildcatMarketController::updateLenderAuthorization() applies a specific lender's current authorization status (set by a borrower) to target markets. Anyone can call this function.

Once the function is executed, it will validate the validity of each target market and then run the WildcatMarketConfig::updateAccountAuthorization() on the target market to update the given lender's current authorization status. If the borrower has authorized the given lender, the lender's approval status will be assigned with AuthRole.DepositAndWithdraw. Otherwise, the lender's approval status will be set to AuthRole.WithdrawOnly -- This design choice allows deauthorized lenders to withdraw their underlying assets from the market.

However, this design choice also allows an attacker to escalate the privilege of their unauthorized account from AuthRole.Null to AuthRole.WithdrawOnly. Consequently, the attacker's unauthorized account can exit asset tokens from a market.

To elaborate on the vulnerability, assuming that John wants to escalate their Sybil account called SybilJohn. John invokes the WildcatMarketController::updateLenderAuthorization() and specifies SybilJohn as the lender argument. Since SybilJohn is not an authorized lender, the _authorizedLenders.contains(SybilJohn) will pass the 'false' value to the WildcatMarketConfig::updateAccountAuthorization().

Because the _isAuthorized parameter would indicate the 'false' value, the WildcatMarketConfig::updateAccountAuthorization() will assign the AuthRole.WithdrawOnly approval status to SybilJohn. In other words, John has successfully escalated its Sybil account.

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol
@>  function updateLenderAuthorization(address lender, address[] memory markets) external { //@audit -- John inputs his Sybil account (SybilJohn) as the lender argument
      for (uint256 i; i < markets.length; i++) {
        address market = markets[i];
        if (!_controlledMarkets.contains(market)) {
          revert NotControlledMarket();
        }
@>      WildcatMarket(market).updateAccountAuthorization(lender, _authorizedLenders.contains(lender)); //@audit -- Since SybilJohn is not an authorized lender, the _authorizedLenders.contains(SybilJohn) will pass the 'false' value to the WildcatMarketConfig::updateAccountAuthorization()
      }
    }

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketConfig.sol
    function updateAccountAuthorization(
      address _account,
      bool _isAuthorized
    ) external onlyController nonReentrant {
      MarketState memory state = _getUpdatedState();
      Account memory account = _getAccount(_account);
      if (_isAuthorized) {
        account.approval = AuthRole.DepositAndWithdraw;
      } else {
@>      account.approval = AuthRole.WithdrawOnly; //@audit -- Because the _isAuthorized == false, SybilJohn's approval status will be assigned with AuthRole.WithdrawOnly (privilege escalation!!)
      }
      _accounts[_account] = account;
      _writeState(state);
      emit AuthorizationStatusUpdated(_account, account.approval);
    }

• John inputs his Sybil account (SybilJohn) as the lender argument: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L182
• Since SybilJohn is not an authorized lender, the _authorizedLenders.contains(SybilJohn) will pass the 'false' value to the WildcatMarketConfig::updateAccountAuthorization(): https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L188
• Because the _isAuthorized == false, SybilJohn's approval status will be assigned with AuthRole.WithdrawOnly (privilege escalation!!): https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L121

Step-by-step asset evacuation and exiting process

Assuming that John is a lender sanctioned by Chainalysis, Bob is trying to execute the WildcatMarketConfig::nukeFromOrbit() to block John from interacting with the market and transfer his market tokens to a locked Escrow.

John can leverage the privilege escalation vulnerability in this report together with the front-running and Sybil attacks to evacuate and exit their underlying assets from the market without a permit. Consider the following exploit scenario.

1. John front runs the execution of the WildcatMarketConfig::nukeFromOrbit() invoked by Bob to invoke the WildcatMarketToken::transfer() to transfer his market tokens to his Sybil account called SybilJohn. John can successfully transfer his tokens to SybilJohn at this step because his approval status has not yet been updated to AuthRole.Blocked due to the front running.

2. However, since SybilJohn is an unauthorized account with approval status == AuthRole.Null by default, SybilJohn cannot exit John's underlying assets from the market.

3. John executes the WildcatMarketController::updateLenderAuthorization() by inputting SybilJohn as the lender argument to escalate SybilJohn's privilege from AuthRole.Null to AuthRole.WithdrawOnly (Please refer to the Explaining how an unauthorized account can escalate its privilege section above for a detailed explanation).

4. With the obtained AuthRole.WithdrawOnly approval status, SybilJohn can burn the market tokens and exit the underlying asset tokens from the market through the invocation of the WildcatMarketWithdrawals::queueWithdrawal() and WildcatMarketWithdrawals::executeWithdrawal() like a regular/unblocked lender.

With this exploit scenario, the market's borrower or Wildcat protocol owner cannot block SybilJohn from exiting John's asset tokens.

Tools Used

Manual Review

Recommended Mitigation Steps

To fix the vulnerability, apply the onlyBorrower modifier to the WildcatMarketController::updateLenderAuthorization(), like the below snippet.

In addition to the recommendation, since only a borrower can authorize or deauthorize lenders in their markets, it still makes sense to apply the onlyBorrower modifier to the updateLenderAuthorization() to fix the vulnerability.

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol
-   function updateLenderAuthorization(address lender, address[] memory markets) external {
+   function updateLenderAuthorization(address lender, address[] memory markets) external onlyBorrower {
      for (uint256 i; i < markets.length; i++) {
        address market = markets[i];
        if (!_controlledMarkets.contains(market)) {
          revert NotControlledMarket();
        }
        WildcatMarket(market).updateAccountAuthorization(lender, _authorizedLenders.contains(lender));
      }
    }

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L79 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L173-L174 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L178 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L96-L97 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L108 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L110 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L165 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L167-L168 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L171 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsEscrow.sol#L38

Vulnerability details

The WildcatMarketConfig::nukeFromOrbit() and the WildcatMarketWithdrawals::executeWithdrawal() create incorrect Escrows for locking the blocked lender's asset tokens (underlying assets) and market tokens.

Impact

A borrower can steal all asset tokens and market tokens of the blocked lender. Eventually, the borrower can burn the stolen market tokens and withdraw all underlying asset tokens from the market.

Proof of Concept

This PoC section is divided into three sub-sections:

• Explaining how the nukeFromOrbit() creates an incorrect Escrow for the market tokens
• Explaining how the executeWithdrawal() creates incorrect Escrows for the market tokens and the asset tokens
• Step-by-step exploit scenarios

Explaining how the nukeFromOrbit() creates an incorrect Escrow for the market tokens

Once a lender gets sanctioned by Chainalysis, anyone can trigger the WildcatMarketConfig::nukeFromOrbit() to block the lender from interacting with the market and transfer all lender's market tokens to an Escrow contract.

The nukeFromOrbit() invokes the WildcatMarketBase::_blockAccount() to block the lender. The _blockAccount() will create the Escrow for holding the blocked lender's market tokens. However, the _blockAccount() will pass the accountAddress and borrower arguments into the WildcatSanctionsSentinel::createEscrow() alternately with the createEscrow()'s required parameters.

Subsequently, the createEscrow()'s borrower parameter will point to the blocked lender, whereas the account parameter will point to the borrower instead. Therefore, the createEscrow() will create the incorrect Escrow contract.

After that, all blocked lender's market tokens will be transferred to the incorrectly generated Escrow, allowing the borrower to execute the WildcatSanctionsEscrow::releaseEscrow() on the Escrow to steal all market tokens.

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketConfig.sol
    function nukeFromOrbit(address accountAddress) external nonReentrant {
      if (!IWildcatSanctionsSentinel(sentinel).isSanctioned(borrower, accountAddress)) {
        revert BadLaunchCode();
      }
      MarketState memory state = _getUpdatedState();
@>    _blockAccount(state, accountAddress); //@audit -- invoke the _blockAccount() to block the sanctioned lender
      _writeState(state);
    }

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketBase.sol
    function _blockAccount(MarketState memory state, address accountAddress) internal {
      Account memory account = _accounts[accountAddress];
      if (account.approval != AuthRole.Blocked) {
        ...

        if (scaledBalance > 0) {
          account.scaledBalance = 0;
          address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
@>          accountAddress, //@audit -- the incorrect Escrow for the market tokens is created
@>          borrower,
            address(this)
          );
          emit Transfer(accountAddress, escrow, state.normalizeAmount(scaledBalance));
@>        _accounts[escrow].scaledBalance += scaledBalance; //@audit -- all blocked lender's market tokens are sent to the Escrow
          emit SanctionedAccountAssetsSentToEscrow(
            accountAddress,
            escrow,
            state.normalizeAmount(scaledBalance)
          );
        }
        _accounts[accountAddress] = account;
      }
    }

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatSanctionsSentinel.sol
    function createEscrow(
@>    address borrower, //@audit -- notice the difference between the function params and the passed arguments
@>    address account,
      address asset
    ) public override returns (address escrowContract) {
      ...

@>    tmpEscrowParams = TmpEscrowParams(borrower, account, asset); //@audit -- now, the borrower == address(blocked lender) and account == address(legit borrower)

@>    new WildcatSanctionsEscrow{ salt: keccak256(abi.encode(borrower, account, asset)) }(); //@audit -- create an incorrect Escrow that the (legit) borrower can execute Escrow::releaseEscrow() to steal the blocked lender's tokens

      ...
    }

• Invoke the _blockAccount() to block the sanctioned lender: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L79
• The incorrect Escrow for the market tokens is created: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L173-L174
• All blocked lender's market tokens are sent to the Escrow: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L178
• Notice the difference between the function params and the passed arguments: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L96-L97
• Now, the borrower == address(blocked lender) and account == address(legit borrower): https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L108
• Create an incorrect Escrow that the (legit) borrower can execute Escrow::releaseEscrow() to steal the blocked lender's tokens: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L110

Explaining how the executeWithdrawal() creates incorrect Escrows for the market tokens and the asset tokens

When a sanctioned lender (but has not been blocked from the market) executes the WildcatMarketWithdrawals::executeWithdrawal(), the function will invoke the WildcatMarketBase::_blockAccount() to block the lender from the market and then transfer all their market tokens to a created Escrow.

In this step, the _blockAccount() will create an incorrect Escrow for the blocked lender's market tokens, allowing the borrower to steal all locked market tokens. For a detailed explanation, please refer to the Explaining how the nukeFromOrbit() creates an incorrect Escrow for the market tokens section above.

Then, the executeWithdrawal() will execute the WildcatSanctionsSentinel::createEscrow() to create an Escrow for holding the blocked lender's asset tokens (from a pending withdrawal request that has expired). Similar to the _blockAccount(), the executeWithdrawal() will pass the accountAddress and borrower arguments into the createEscrow() alternately with its required parameters.

For this reason, the createEscrow()'s borrower parameter will point to the blocked lender, whereas the account parameter will point to the borrower unintentionally. Hence, the createEscrow() will create the incorrect Escrow contract.

Lastly, all blocked lender's asset tokens (from a pending withdrawal request that has expired) will be transferred to the incorrectly created Escrow, allowing the borrower to execute the WildcatSanctionsEscrow::releaseEscrow() on the Escrow to steal all asset tokens.

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketWithdrawals.sol
    function executeWithdrawal(
      address accountAddress,
      uint32 expiry
    ) external nonReentrant returns (uint256) {
      ...

      if (IWildcatSanctionsSentinel(sentinel).isSanctioned(borrower, accountAddress)) {
@>      _blockAccount(state, accountAddress); //@audit -- invoke the _blockAccount() to block the sanctioned lender
        address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
@>        accountAddress, //@audit -- the incorrect Escrow for the asset tokens (underlying assets) is created
@>        borrower,
          address(asset)
        );
@>      asset.safeTransfer(escrow, normalizedAmountWithdrawn); //@audit -- all blocked lender's asset tokens (from a pending withdrawal request that has expired) are sent to the Escrow
        emit SanctionedAccountWithdrawalSentToEscrow(
          accountAddress,
          escrow,
          expiry,
          normalizedAmountWithdrawn
        );
      } else {
        ...
      }

      ...
    }

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketBase.sol
    function _blockAccount(MarketState memory state, address accountAddress) internal {
      Account memory account = _accounts[accountAddress];
      if (account.approval != AuthRole.Blocked) {
        ...

        if (scaledBalance > 0) {
          account.scaledBalance = 0;
          address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
@>          accountAddress, //@audit -- the incorrect Escrow for the market tokens is created
@>          borrower,
            address(this)
          );
          emit Transfer(accountAddress, escrow, state.normalizeAmount(scaledBalance));
@>        _accounts[escrow].scaledBalance += scaledBalance; //@audit -- all blocked lender's market tokens are sent to the Escrow
          emit SanctionedAccountAssetsSentToEscrow(
            accountAddress,
            escrow,
            state.normalizeAmount(scaledBalance)
          );
        }
        _accounts[accountAddress] = account;
      }
    }

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatSanctionsSentinel.sol
    function createEscrow(
@>    address borrower, //@audit -- notice the difference between the function params and the passed arguments
@>    address account,
      address asset
    ) public override returns (address escrowContract) {
      ...

@>    tmpEscrowParams = TmpEscrowParams(borrower, account, asset); //@audit -- now, the borrower == address(blocked lender) and account == address(legit borrower)

@>    new WildcatSanctionsEscrow{ salt: keccak256(abi.encode(borrower, account, asset)) }(); //@audit -- create an incorrect Escrow that the (legit) borrower can execute Escrow::releaseEscrow() to steal the blocked lender's tokens

      ...
    }

• Invoke the _blockAccount() to block the sanctioned lender: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L165
• The incorrect Escrow for the asset tokens (underlying assets) is created: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L167-L168
• All blocked lender's asset tokens (from a pending withdrawal request that has expired) are sent to the Escrow: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L171
• The incorrect Escrow for the market tokens is created: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L173-L174
• All blocked lender's market tokens are sent to the Escrow: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L178
• Notice the difference between the function params and the passed arguments: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L96-L97
• Now, the borrower == address(blocked lender) and account == address(legit borrower): https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L108
• Create an incorrect Escrow that the (legit) borrower can execute Escrow::releaseEscrow() to steal the blocked lender's tokens: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L110

Step-by-step exploit scenarios

There are two exploit scenarios based on the sanctioned lender (victim)'s actions prior to the sanction event.

1. If the sanctioned lender (victim) has no pending withdrawal request:

   1. A borrower (attacker) executes the WildcatMarketConfig::nukeFromOrbit() to block the lender and transfer all lender's market tokens to the incorrectly created Escrow.

   2. The borrower invokes the WildcatSanctionsEscrow::releaseEscrow() on the Escrow to steal the locked market tokens. Because the account parameter will point to the borrower (who is not sanctioned by Chainalysis), the check for the release authorization by the canReleaseEscrow() will be passed (more refs: #1 and #2). The market tokens will be transferred to the borrower.

   3. The borrower executes the WildcatMarketController::authorizeLenders() to authorize themselves as a lender and then calls the WildcatMarketController::updateLenderAuthorization() to apply the lender authorization to the target market.

   4. The borrower triggers the WildcatMarketWithdrawals::queueWithdrawal() to create a pending request for withdrawing (underlying) asset tokens by burning the stolen market tokens for exchange. As a result of Step 3, the borrower now becomes a legitimate lender with the approval status == AuthRole.DepositAndWithdraw. Therefore, the check for the withdrawal authorization by the _getAccountWithRole() will be passed.

   5. Once the pending withdrawal request has expired, the borrower triggers the WildcatMarketWithdrawals::executeWithdrawal() to withdraw the blocked lender's (underlying) asset tokens from the market. The stolen asset tokens will be transferred to the borrower.

2. If the sanctioned lender (victim) has previously executed some pending withdrawal requests before being sanctioned:

   1. A borrower (attacker) executes the WildcatMarketWithdrawals::executeWithdrawal() to block the lender (if necessary) and transfer both the market tokens and underlying asset tokens (from a pending withdrawal request that has expired) of the blocked lender to the incorrectly created Escrows (There will be 2 Escrows -- one for the market tokens and another for the asset tokens).

   2. The borrower invokes the WildcatSanctionsEscrow::releaseEscrow() on both Escrows to steal the locked market tokens and the underlying asset tokens. Because the account parameter will point to the borrower (who is not sanctioned by Chainalysis), the check for the release authorization by the canReleaseEscrow() will be passed (more refs: #1 and #2). Finally, both the market tokens and asset tokens will be transferred to the borrower. At this step, the borrower has successfully stolen certain asset tokens. To steal the remaining tokens, perform the next step.

   3. The borrower must burn the stolen market tokens for the underlying asset tokens via the process of executing the WildcatMarketWithdrawals::queueWithdrawal() and WildcatMarketWithdrawals::executeWithdrawal() similar to Steps 1.3 - 1.5 above.

Tools Used

Manual Review

Recommended Mitigation Steps

To fix the vulnerability, swap the passing arguments (accountAddress and borrower) in the WildcatMarketBase::_blockAccount() and in the WildcatMarketWithdrawals::executeWithdrawal(), like the snippet below.

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketBase.sol
    function _blockAccount(MarketState memory state, address accountAddress) internal {
      Account memory account = _accounts[accountAddress];
      if (account.approval != AuthRole.Blocked) {
        ...

        if (scaledBalance > 0) {
          account.scaledBalance = 0;
          address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
-           accountAddress,
-           borrower,
+           borrower,
+           accountAddress,
            address(this)
          );
          emit Transfer(accountAddress, escrow, state.normalizeAmount(scaledBalance));
          _accounts[escrow].scaledBalance += scaledBalance;
          emit SanctionedAccountAssetsSentToEscrow(
            accountAddress,
            escrow,
            state.normalizeAmount(scaledBalance)
          );
        }
        _accounts[accountAddress] = account;
      }
    }

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketWithdrawals.sol
    function executeWithdrawal(
      address accountAddress,
      uint32 expiry
    ) external nonReentrant returns (uint256) {
      ...

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

      ...
    }

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L487 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L156 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L67 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L68

Vulnerability details

The WildcatMarketController::setAnnualInterestBips() applies the annualInterestBips parameter (lending interest rate) set by a borrower without asserting it with the controller's MinimumAnnualInterestBips/MaximumAnnualInterestBips constraints, allowing a borrower to set a zero interest for their borrowing.

Proof of Concept

A borrower can execute the WildcatMarketController::setAnnualInterestBips() to modify their market's annualInterestBips parameter (lending interest rate). Upon execution, the function will invoke the WildcatMarketConfig::setAnnualInterestBips() on the target market to adjust the annualInterestBips.

However, the annualInterestBips inputted by the borrower will be applied to the target market without asserting with the controller's MinimumAnnualInterestBips and MaximumAnnualInterestBips constraints.

Hence, the borrower can set a zero interest for their borrowing. In other words, all lenders will receive 0% lending interest.

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol
    function setAnnualInterestBips(
      address market,
      uint16 annualInterestBips
    ) external virtual onlyBorrower onlyControlledMarket(market) {
      // If borrower is reducing the interest rate, increase the reserve
      // ratio for the next two weeks.
      if (annualInterestBips < WildcatMarket(market).annualInterestBips()) {
        TemporaryReserveRatio storage tmp = temporaryExcessReserveRatio[market];

        if (tmp.expiry == 0) {
          tmp.reserveRatioBips = uint128(WildcatMarket(market).reserveRatioBips());

          // Require 90% liquidity coverage for the next 2 weeks
          WildcatMarket(market).setReserveRatioBips(9000);
        }

        tmp.expiry = uint128(block.timestamp + 2 weeks);
      }

@>    WildcatMarket(market).setAnnualInterestBips(annualInterestBips);
    }

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketConfig.sol
    function setAnnualInterestBips(uint16 _annualInterestBips) public onlyController nonReentrant {
      MarketState memory state = _getUpdatedState();

      if (_annualInterestBips > BIP) {
        revert InterestRateTooHigh();
      }

@>    state.annualInterestBips = _annualInterestBips;
      _writeState(state);
      emit AnnualInterestBipsUpdated(_annualInterestBips);
    }

• https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L487

• https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L156

Impact

A borrower can set a zero interest for their borrowing. In other words, all lenders will receive 0% lending interest.

Tools Used

Manual Review

Recommended Mitigation Steps

Assert the annualInterestBips parameter against the controller's MinimumAnnualInterestBips and MaximumAnnualInterestBips constraints in the WildcatMarketController::setAnnualInterestBips(), like the below snippet.

    // FILE: https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol
    function setAnnualInterestBips(
      address market,
      uint16 annualInterestBips
    ) external virtual onlyBorrower onlyControlledMarket(market) {
+     assertValueInRange(
+       annualInterestBips,
+       MinimumAnnualInterestBips,
+       MaximumAnnualInterestBips,
+       AnnualInterestBipsOutOfBounds.selector
+     );

      // If borrower is reducing the interest rate, increase the reserve
      // ratio for the next two weeks.
      if (annualInterestBips < WildcatMarket(market).annualInterestBips()) {
        TemporaryReserveRatio storage tmp = temporaryExcessReserveRatio[market];

        if (tmp.expiry == 0) {
          tmp.reserveRatioBips = uint128(WildcatMarket(market).reserveRatioBips());

          // Require 90% liquidity coverage for the next 2 weeks
          WildcatMarket(market).setReserveRatioBips(9000);
        }

        tmp.expiry = uint128(block.timestamp + 2 weeks);
      }

      WildcatMarket(market).setAnnualInterestBips(annualInterestBips);
    }

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L14 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L42 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsEscrow.sol#L26 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L75 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L94 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L164

Vulnerability details

The chainalysisSanctionsList parameter of the WildcatSanctionsSentinel contract is an immutable variable. It will be unchangeable if Chainalysis's SanctionsList contract is down, affecting several essential functions in the Wildcat protocol to malfunction.

Proof of Concept

The vulnerability flagged in this report is not about any centralization or trust risks of Chainalysis; it is only about the risks from human errors or management mistakes. However, relying on the third party's security (Chainalysis) should not be the best security idea of the Wildcat protocol.

In the WildcatSanctionsSentinel contract, the chainalysisSanctionsList is declared immutable, pointing to Chainalysis's SanctionsList contract. Therefore, no one can update it if necessary after the Sentinel contract is deployed.

The chainalysisSanctionsList parameter is employed in the critical WildcatSanctionsSentinel::isSanctioned() to query the sanction status of all lenders in every Market contract throughout the protocol.

I noticed that Chainalysis's SanctionsList contract implements the critical single-step functions: renounceOwnership() and transferOwnership(). If one of them is somehow executed incorrectly, that could brick the SanctionsList contract eternally.

Consequently, the Wildcat protocol's functions will be directly damaged since no one can update the chainalysisSanctionsList parameter.

    contract WildcatSanctionsSentinel is IWildcatSanctionsSentinel {
      ...

@>    address public immutable override chainalysisSanctionsList;

      ...

      /**
      * @dev Returns boolean indicating whether `account` is sanctioned
      *      on Chainalysis and that status has not been overridden by
      *      `borrower`.
      */
      function isSanctioned(address borrower, address account) public view override returns (bool) {
        return
          !sanctionOverrides[borrower][account] &&
@>        IChainalysisSanctionsList(chainalysisSanctionsList).isSanctioned(account);
      }

      ...
    }

• https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L14

• https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L42

Impact

The vulnerability flagged in this report is not about any centralization or trust risks of Chainalysis; it is only about the risks from human errors or management mistakes. However, relying on the third party's security (Chainalysis) should not be the best security idea of the Wildcat protocol.

The following lists essential functions that require the oracle data sourced from Chainalysis's SanctionsList contract.

1. WildcatSanctionsEscrow contract

   1. WildcatSanctionsEscrow::canReleaseEscrow()

2. WildcatMarket contract

   1. WildcatMarketConfig::nukeFromOrbit()
   2. WildcatMarketConfig::stunningReversal()
   3. WildcatMarketWithdrawals::executeWithdrawal()

If Chainalysis's SanctionsList contract is down, the above functions could malfunction. For instance, no one, including the borrower and the Wildcat protocol owner, can block any sanctioned lenders from interacting with their markets.

Moreover, the bricking of the SanctionsList will affect all deployed Market contracts, MarketController contracts, and MarketControllerFactory contracts as they point to the singleton Sentinel contract.

Tools Used

Manual Review

Recommended Mitigation Steps

The chainalysisSanctionsList parameter in the WildcatSanctionsSentinel contract should be a state variable allowing authorized users (e.g., the protocol owner) to update it when necessary.

Since the Sentinel is a singleton contract, updating the chainalysisSanctionsList parameter at the Sentinel would be effective for all deployed Market contracts, MarketController contracts, and MarketControllerFactory contracts discussed previously.

Assessed type

Oracle