The Wildcat Protocol - HChang26's results

Lines of code

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

Vulnerability details

Impact

The platform experiences a critical issue where markets are unable to be closed as intended. This leads to undesirable consequences, including the continuous ability for lenders to make deposits and collect interest, even when it should not be allowed.

Proof of Concept

The issue at hand is in direct contradiction to the documentation, which indicates that borrowers should have the ability to close a market when necessary. Per the documentation:

In the event that a borrower has finished utilising the funds for the purpose that the market was set up to facilitate (or if lenders are choosing not to withdraw their assets and the borrower is paying too much interest on assets that have been re-deposited to the market), the borrower can close a market at will.

closeMarket() can only be called by WildcatMarketController due to onlyController modifier.

  function closeMarket() external onlyController nonReentrant {

However, closeMarket() is not implemented anywhere on WildcatMarketController.sol. This function is not accessible to anyone and lenders can continue to deposit() funds and borrower will have to pay interest on funds they are not utilizing.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement _closeMarket() on WilcatMarketController.sol

  function _closeMarket() external onlyBorrower {
    closeMarket();
  }

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketToken.sol#L64 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketConfig.sol#L74 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol#L182 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketConfig.sol#L112 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketWithdrawals.sol#L77

Vulnerability details

Impact

Users designated as "sanctioned" by the Chainalysis oracle can effectively circumvent their sanctioned status, enabling them to accrue interest and withdraw funds without restriction.

Proof of Concept

The Chainalysis oracle identifies users as "sanctioned" based on specific criteria. However, the market does not immediately register this status until a user initiates the nukeFromOrbit() function. This function transfers a sanctioned user's market tokens into an escrow. These tokens are effectively locked until the user's sanctioned status is overridden.

The existing security mechanism fails to entirely prevent sanctioned users from accessing their tokens. The core issue lies within the _transfer() function in WildcatMarketToken.sol. A sanctioned user can easily front-run the nukeFromOrbit() function and then use transfer() to send their tokens to another one of their accounts. This manipulation bypasses the security checks since the account.approval status is not yet set to AuthRole.Blocked.

  function _transfer(address from, address to, uint256 amount) internal virtual {
    MarketState memory state = _getUpdatedState();
    uint104 scaledAmount = state.scaleAmount(amount).toUint104();

    if (scaledAmount == 0) {
      revert NullTransferAmount();
    }

    Account memory fromAccount = _getAccount(from);
    fromAccount.scaledBalance -= scaledAmount;
    _accounts[from] = fromAccount;

    Account memory toAccount = _getAccount(to);
    toAccount.scaledBalance += scaledAmount;
    _accounts[to] = toAccount;

    _writeState(state);
    emit Transfer(from, to, amount);
  }

Once this transfer is executed, the user can call updateLenderAuthorization(), which triggers updateAccountAuthorization(). This, in turn, permits the user to access queueWithdrawal(). The outcome is that the "sanctioned" user can effectively avoid being banned from the market, continuing to accumulate interest and withdraw funds as if they were a regular, unsanctioned user.

  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;
    }
    _accounts[_account] = account;
    _writeState(state);
    emit AuthorizationStatusUpdated(_account, account.approval);
  }

Tools Used

Manual Review

Recommended Mitigation Steps

import { WildcatSanctionsSentinel } from './WildcatSanctionsSentinel.sol' into WildcatMarketToken.sol initialize it with correct address and add isSanctioned() checks for both from and to.

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketBase.sol#L163 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketConfig.sol#L74 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/libraries/MarketState.sol#L87 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/libraries/MarketState.sol#L123 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/libraries/MarketState.sol#L138

Vulnerability details

Impact

The _blockAccount() function fails to accurately adjust state.scaledTotalSupply, resulting in a cascade of accounting inaccuracies. This miscalculation leads to inflated figures for borrowable assets, liquidity requirements, and total debt

Proof of Concept

The Chainalysis oracle designates users as "sanctioned" if they meet certain criteria. The key issue revolves around the _blockAccount() function, which can be invoked by anyone through the nukeFromOrbit() method:

In this process, account.scaledBalance is reset to 0, and a corresponding escrow is created. The scaledBalance value is then transferred into the escrow's _accounts[escrow].scaledBalance. It's important to note that state.scaledTotalSupply for the entire market is not reduced accordingly.

  function _blockAccount(MarketState memory state, address accountAddress) internal {
    Account memory account = _accounts[accountAddress];
    if (account.approval != AuthRole.Blocked) {
      uint104 scaledBalance = account.scaledBalance;
      account.approval = AuthRole.Blocked;
      emit AuthorizationStatusUpdated(accountAddress, AuthRole.Blocked);

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

The absence of an adjustment to state.scaledTotalSupply triggers a series of internal accounting inaccuracies. It inflates critical metrics such as liquidityRequired(), totalDebts(), and borrowableAssets().

Tools Used

Manual Review

Recommended Mitigation Steps

  function _blockAccount(MarketState memory state, address accountAddress) internal {
    Account memory account = _accounts[accountAddress];
    if (account.approval != AuthRole.Blocked) {
      uint104 scaledBalance = account.scaledBalance;
      account.approval = AuthRole.Blocked;
      emit AuthorizationStatusUpdated(accountAddress, AuthRole.Blocked);

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

Assessed type

Math