The Wildcat Protocol - Drynooo's results

Lines of code

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

Vulnerability details

Impact

Both the closeMarket and setMaxTotalSupply functions have the onlyController restriction. However, it is impossible to invoke these two functions in the WildcatMarketController contract. Consequently, this situation prevents borrowers from closing a market or adjusting state.maxTotalSupply. As a result, there is no way to close the market even if the borrower wants to do so, resulting in constant interest accruing and losses to the borrower.

Proof of Concept

Searching for these two functions throughout the project did not find a call to either function somewhere. https://github.com/search?q=repo%3Acode-423n4%2F2023-10-wildcat%20closeMarket&type=code https://github.com/search?q=repo%3Acode-423n4%2F2023-10-wildcat+setMaxTotalSupply&type=code

Although these two functions are called in the test, that's due to forcing the caller to be set to controller, which is clearly not feasible in a real-world situation.

function test_closeMarket_TransferRemainingDebt() external asAccount(address(controller)) {
    // Borrow 80% of deposits then request withdrawal of 100% of deposits
    _depositBorrowWithdraw(alice, 1e18, 8e17, 1e18);
    startPrank(borrower);
    asset.approve(address(market), 8e17);
    stopPrank();
    vm.expectEmit(address(asset));
    emit Transfer(borrower, address(market), 8e17);
    market.closeMarket();
  }

modifier asAccount(address prank) {
    startPrank(prank);
    _;
    stopPrank();
  }

Tools Used

Vscode

Recommended Mitigation Steps

It is recommended to add the call logic for the relevant functions in the WildcatMarketController.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

If sender==from when calling transferFrom function, it will also deduct allowances, which should not be deducted. In many protocols only transferFrom is used, this design will break the compatibility with many protocols.

Proof of Concept

The trasnferFrom method always uses allowance even if spender = from. https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketToken.sol#L41-L57

  function transferFrom(
    address from,
    address to,
    uint256 amount
  ) external virtual nonReentrant returns (bool) {
    uint256 allowed = allowance[from][msg.sender];

    // Saves gas for unlimited approvals.
    if (allowed != type(uint256).max) {
      uint256 newAllowance = allowed - amount;
      _approve(from, msg.sender, newAllowance);
    }

    _transfer(from, to, amount);

    return true;
  }

Tools Used

Vscode

Recommended Mitigation Steps

Only use allowance when spender != from

Assessed type

ERC20