The Wildcat Protocol - 0xComfyCat'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/WildcatMarketController.sol#L32-L516

Vulnerability details

Impact

When borrower is done borrowing from market, they should be able to close the market to avoid paying interest further. The closeMarket function of market contract must be called by it's market controller. But in current market controller implementation, the function that suppose to perform a call to closeMarket doesn't exist. Therefore borrower will not be able to close the market.

Proof of Concept

Search through the code repository and no such function exist.

Tools Used

Manual review

Recommended Mitigation Steps

Implement close market functionality on market controller contract

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Market's maxTotalSupply parameter is set during market creation and can be updated via market config setMaxTotalSupply function which is marked with onlyController modifier. But in current market controller implementation, the function that suppose to perform a call to market's setMaxTotalSupply doesn't exist. Therefore borrower will not be able to update market maxTotalSupply parameter after the market has been deployed.

Proof of Concept

Search through the code repository and no such function exist.

Tools Used

Manual review

Recommended Mitigation Steps

Implement such functionality on market controller contract

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketConfig.sol#L118-L122 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol#L182-L190

Vulnerability details

Impact

Per documentation "However, it is worth noting that withdrawal requests (and subsequent claims/redemptions) can only be generated by addresses that have been approved for the controller contract of a given market" and stated invariant "The assets of a market should never be able to be withdrawn by anyone that is not the borrower or a lender with either the WithdrawOnly or DepositAndWithdraw role."

Only address with WithdrawOnly role and above should be able to queue withdrawal. The role check is performed within queueWithdrawal correctly. However, arbitrary address with default AuthRole Null that is not an approved lender can obtain WithdrawOnly by calling controller updateLenderAuthorization. Thus, anyone can queue a withdrawal and violate the invariant.

Proof of Concept

Controller updateLenderAuthorization makes an updateAccountAuthorization call to the specified market. Sending a boolean flag which determine if the lender is authorized or not.

  function updateLenderAuthorization(address lender, address[] memory markets) external {
    for (uint256 i; i < markets.length; i++) {
      ...
      WildcatMarket(market).updateAccountAuthorization(lender, _authorizedLenders.contains(lender));
    }
  }

Market's updateAccountAuthorization then grant lender role based on the flag.

  function updateAccountAuthorization(
    address _account,
    bool _isAuthorized
  ) external onlyController nonReentrant {
    ...
    if (_isAuthorized) {
      account.approval = AuthRole.DepositAndWithdraw;
    } else {
      account.approval = AuthRole.WithdrawOnly;
    }
    ...
  }

However, even the account is not authorized, the function will grant them WithdrawOnly role by default.

Following test case showed that bob with initial Null role can obtain WithdrawOnly role and queue a withdrawal. The test can be paste into WithdrawalsTest contract and run as-is.

    function test_AnyoneCanWithdraw() public {
        // Bob failed to queue withdrawal
        // Copied from test_queueWithdrawal_NotApprovedLender
        _deposit(alice, 1e18);
        vm.prank(alice);
        market.transfer(bob, 1e18);
        vm.startPrank(bob);
        vm.expectRevert(IMarketEventsAndErrors.NotApprovedLender.selector);
        market.queueWithdrawal(1e18);
        // End copy

        // Bob can't queue a withdrawal because his role is null
        assertEq(uint8(market.getAccountRole(bob)), uint8(AuthRole.Null));
        // So his withdrawal amount should be 0
        AccountWithdrawalStatus memory status = market
            .getAccountWithdrawalStatus(
                bob,
                uint32(block.timestamp + parameters.withdrawalBatchDuration)
            );
        assertEq(status.scaledAmount, 0);

        // We update bob role to WithdrawOnly via `updateLenderAuthorization`
        address[] memory markets = new address[](1);
        markets[0] = address(market);
        controller.updateLenderAuthorization(bob, markets);
        // Now bob has the WithdrawOnly role
        assertEq(
            uint8(market.getAccountRole(bob)),
            uint8(AuthRole.WithdrawOnly)
        );

        // Bob tries to queue a withdrawal and succeeds
        vm.prank(bob);
        market.queueWithdrawal(1e18);
        // His withdrawal is queued
        status = market.getAccountWithdrawalStatus(
            bob,
            uint32(block.timestamp + parameters.withdrawalBatchDuration)
        );
        assertEq(status.scaledAmount, 1e18);
    }

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

On updateAccountAuthorization grant role Null by default if lender is not authorized.

Assessed type

Access Control

Low

It is not trivial to withdraw 100%

It is not trivial to withdraw 100% because the input amount of queueWithdrawal will be less than actual market token balance of lender when the transaction is executed due to interest accrual. Consider add function queueWithdrawalAll

If borrower set reserve ratio higher than 90% they can reduce it to 90% consistently by calling setAnnualInterestBips with lower interest

setAnnualInterestBips always fixed reserveRatio to 90%. If somehow borrower deploy a market with 95% reserveRatio they can lower it to 90% consistently by calling setAnnualInterestBips every 2 weeks.

Non-critical

annualInterestBips is too restrictive

annualInterestBips is hard-capped at 100% in market config contract setReserveRatioBips. Borrower might want to deploy a market with higher interest rate to attract lenders.