The Wildcat Protocol - 0xAsen's results

Lines of code

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

Vulnerability details

Impact

In function _blockAccount in WildcatMarketBase.sol, the parameters for createEscrow are passed in the wrong order leading to unintended behaviour of the newly created Escrow contract and opening up a lot of issues.

Let's see the part of the _blockAccount function where we call createEscow:

 function _blockAccount(MarketState memory state, address accountAddress) internal {
  
      if (scaledBalance > 0) {
        account.scaledBalance = 0;
        address escrow = IWildcatSanctionsSentinel(sentinel).createEscrow(
          accountAddress,
          borrower,
          address(this)
        );
  }

while the createEscrow function expects the parameters in the following order:

function createEscrow(
    address borrower,
    address account,
    address asset
  ) public override returns (address escrowContract) {}

Instead of borrower, we're passing the account and vice versa. This leads to multiple issues like the supposed blocked lender being able to release funds. The blockAccount function is used in nukeFromOrbit and results in not really being able to block someone.

Proof of Concept

Here is a POC that I added to WildcatMarketConfig.t.sol: You need to add this to the test file before that:

import { WildcatSanctionsEscrow, IWildcatSanctionsEscrow } from 'src/WildcatSanctionsEscrow.sol';

contract MockWildcatArchController {
  mapping(address market => bool) public isRegisteredMarket;

  function setIsRegisteredMarket(address market, bool isRegistered) external {
    isRegisteredMarket[market] = isRegistered;
  }
}

And the test:

   function test_nukeFromOrbitIssue() public {
    //deploying mock archController
    MockWildcatArchController archController;
    archController = new MockWildcatArchController();
    
    //deploying mock SanctionsSentinel
    sanctionsSentinel = new MockSanctionsSentinel(address(archController));
    
    //set this address as registered market
    archController.setIsRegisteredMarket(address(this), true);

    //deposit in market
    vm.prank(alice);
    market.deposit(1e18);

    //sanction alice
    sanctionsSentinel.sanction(alice);
      
    //nuke it and assert it's blocked
    market.nukeFromOrbit(alice);
    assertEq(
      uint(market.getAccountRole(alice)),
      uint(AuthRole.Blocked),
      'account role should be Blocked'
    );
    
    //create the escrow with the param order in blockAccount
    WildcatSanctionsEscrow escrow = WildcatSanctionsEscrow(
      sanctionsSentinel.createEscrow(alice, borrower, address(market))
    );

    //should be false as Alice is sanctioned by borrower
    //but returns true because what it checks actually is if borrower is sanctioned by Alice
    console.log("canReleaseEscrow: ", escrow.canReleaseEscrow());
  }

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Pass the parameters correctly:

function createEscrow(address borrower, address account, address asset)

Assessed type

Error