The Wildcat Protocol - SovaSlava's results

Lines of code

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

Vulnerability details

Impact

From 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.
This is a special case of reducing the APR (with the associated increased reserve rate that accompanies it). When a market is closed, sufficient assets must be repaid to increase the reserve ratio to 100%, after which interest ceases to accrue and no further parameter adjustment or borrowing is possible. The only thing possible to do in a closed market is for the lenders to file withdrawal requests and exit.

But owner could not close market, because there is not function in WildcatMarketController, which could call market.closeMarket(). Borrower could not call market.closeMarket() directly, because this function has modifier onlyController, which allow calls only from WildcatMarketController contract

Proof of Concept

Just search phrase "closeMarket" in WildcatMarketController.sol file. You will not find it.

Tools Used

Manual review

Recommended Mitigation Steps

Add function to WildcatMarketController.sol for borrower, which allow him call market.closeMarket()

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketToken.sol#L36-L57

Vulnerability details

Impact

If a lender knows that he is blacklisted in chainalysis Sanctions List and wants to withdraw tokens, but does not want them to go to an escrow contract, he can do:

1. Transfer tokens to another lender on this market(his friend) - lender2.
2. Lender2 call withdraw function and get assets tokens.
3. After that, lender2 transfer asset to his friend - lender1.

Proof of Concept

  function testPOCEscapeSanction() public {
    vm.startPrank(borrower);
    address[] memory lenders = new address[](2);
    lenders[0] = alice;
    lenders[1] = monika;
    controller.authorizeLenders(lenders);
    vm.stopPrank();
    _deposit(alice, 1e18);
    _deposit(monika, 1e18);
    sanctionsSentinel.sanction(alice);
    vm.prank(alice);
    market.transfer(monika,1e18);
    vm.startPrank(monika);
    market.queueWithdrawal(1e18);
    uint256 expiry = block.timestamp + parameters.withdrawalBatchDuration;
    fastForward(parameters.withdrawalBatchDuration);
    market.executeWithdrawal(monika, uint32(expiry)); // creating escrow
    asset.transfer(alice, 1e18);    
  }

Tools Used

Manual review

Recommended Mitigation Steps

Add check in function transfer and transferFrom, that msg.sender dont included in chainalysisSanctionsList.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L95-L99 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L166-L170 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketBase.sol#L172-L176

Vulnerability details

Impact

Due to the incorrect order of arguments in createEscrow function in WildcatSanctionsSentinel contract, lender's tokens/assets is sent directly to the borrower. The variable account is assigned the borrower's address, while the variable borrower is assigned the creditor's address.

Example: The lender gets added to the chainalysis sanctions list, and someone calls the nukeFromOrbit() function. This function calls internal function _blockAccount(state, lendersAddress).

  function _blockAccount(MarketState memory state, address accountAddress) internal {
      ...
      if (scaledBalance > 0) {
        account.scaledBalance = 0;
        // @audit-issue Wrong arguments order.
        address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
          accountAddress,    // <---- correct variable should be borrower
          borrower,          // <---- correct variable should be accountAddress(lender)
          address(this)
        );
        emit Transfer(accountAddress, escrow, state.normalizeAmount(scaledBalance));
        _accounts[escrow].scaledBalance += scaledBalance;
      ...
    }
  }

// createEscrow function
function createEscrow(
    address borrower,  <--- account's address was passed instead borrower's
    address account,   <--- borrower's address was passed instead account's
    address asset
  ) public override returns (address escrowContract) {
     ....
    tmpEscrowParams = TmpEscrowParams(borrower, account, asset);
    new WildcatSanctionsEscrow{ salt: keccak256(abi.encode(borrower, account, asset)) }();

// constructor of WildcatSanctionsEscrow contract
 constructor() {
    sentinel = msg.sender;
    (borrower, account, asset) = WildcatSanctionsSentinel(sentinel).tmpEscrowParams();
 }

After the escrow contract is created, anyone can call the WildcatSanctionsEscrow.releaseEscrow() function. This function call internal function canReleaseEscrow() and check, that the return value of this function is always true. Function canReleaseEscrow() sends borrower and account addresses to isSanctioned function. The problem is that the values in these variables are swapped with each other.

 function canReleaseEscrow() public view override returns (bool) {
        // send lender's address as first argument!
        // send borrower's address as second argument!
    return !WildcatSanctionsSentinel(sentinel).isSanctioned(borrower, account);
  }

WildcatSanctionsSentinel.sol

  function isSanctioned(address borrower, address account) public view override returns (bool) {
    return
      !sanctionOverrides[borrower][account] &&
      IChainalysisSanctionsList(chainalysisSanctionsList).isSanctioned(account);
  }

This function will return false, because in reality, the check will be performed with incorrect addresses (because they are swapped with each other).

sanctionOverrides[borrower][account] -> sanctionOverrides[LENDER][BORROWER] -> False
IChainalysisSanctionsList(chainalysisSanctionsList).isSanctioned(BORROWER) -> False
False && False -> False

Look at releaseEscrow function

 function releaseEscrow() public override {
  
    if (!canReleaseEscrow()) revert CanNotReleaseEscrow(); <------ canReleaseEscrow return always true

    uint256 amount = balance();

    IERC20(asset).transfer(account, amount); <---- remember, in account variable stored borrower address

    emit EscrowReleased(account, asset, amount);
  }

So tokens will be transfered to borrower.

Proof of Concept

Create file transferFromEscrowToWrongAccount.sol in test/market/

// SPDX-License-Identifier: MIT
pragma solidity >=0.8.20;
import '../BaseMarketTest.sol';
import { WildcatSanctionsEscrow, IWildcatSanctionsEscrow } from 'src/WildcatSanctionsEscrow.sol';


contract WildcatMarketConfigTest is BaseMarketTest {
    event EscrowReleased(address indexed account, address indexed asset, uint256 amount);


  function test_transferFromEscrowToWrongAccount() public {
    _deposit(alice, 1e18);
    sanctionsSentinel.sanction(alice);
    address escrow = sanctionsSentinel.getEscrowAddress(alice, borrower, address(market));
    market.nukeFromOrbit(alice);
    vm.expectEmit(address(escrow));
    // Token transfers to borrower, not to Alice
    emit EscrowReleased(borrower, address(market), 1e18);
    IWildcatSanctionsEscrow(escrow).releaseEscrow();
  }

}

Tools Used

Manual review

Recommended Mitigation Steps

Change order or function arguments in WildcatSanctionsSentinel.createEscrow()

function createEscrow(
-    address borrower,
-    address account,
+    address account,
+    address borrower
    address asset
  ) public override returns (address escrowContract) {
    if (!IWildcatArchController(archController).isRegisteredMarket(msg.sender)) {
      revert NotRegisteredMarket();
    }
....

Assessed type

Other