The Wildcat Protocol - josephdara's results

Lines of code

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

Vulnerability details

Impact

The function closeMarket() sets the APR to 0% and does the necessary fund transfers. This function implements a modifier onlyController, this requires that only the controller can make this call. However the controller contract has no method that calls the function closeMarket().

Proof of Concept

  modifier onlyController() {
    if (msg.sender != controller) revert NotController();
    _;
  }

 function closeMarket() external onlyController nonReentrant {}

In the Market Controller contract available at https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol, there exists no function to call the closeMarket()

Tools Used

Manual Review

Recommended Mitigation Steps

Add a function in the Market Controller contract that calls WildcatMarket.closeMarket()

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatSanctionsSentinel.sol#L100-L102 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketWithdrawals.sol#L164-L170

Vulnerability details

Impact

Markets can be added and removed from the ArchController by the controller and owner respectively. Removal of a market from the ArchController is expected, however this creates an issue for sanctioned acccount.

Proof of Concept

Sanctioned accounts are unable to make a withdrawal because the function createEscrow checks if the market is registered.

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

This causes a DOS for addresses with a sanction as the transaction would always revert and users will not get their funds. After a discussion with the sponsor, it was said that the check will be removed from the sanctions sentinel to keep it from affecting withdrawal.

Tools Used

Manual Review

Recommended Mitigation Steps

As said by the sponsor, the check would be removed to allow santioned users access their funds.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L67-L68 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L468-L488 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketConfig.sol#L149-L159

Vulnerability details

Impact

When markets are deployed using the Factory deployControllerAndMarket() or with the controllerdeployMarket(), a check is performed on the AnnualInterestBips It is ensured that the APR is always less than maximumAnnualInterestBips preset in the factory and greater than the MinimumAnnualInterestBips However, a borrower can increase or reduce the market's APR to any value using the WildcatMarketController.setAnnualInterestBips() It can be set to zero, or any amount greater than the allowed maximum from the Factory contract. After a discussion with the spponsor, this was confirmed to be an issue.

Proof of Concept

The function setAnnualInterestBips() does no checks to ensure the limiters on the new annualInterestBips.

function setAnnualInterestBips(
    address market,
    uint16 annualInterestBips
  ) external virtual onlyBorrower onlyControlledMarket(market) {
    // If borrower is reducing the interest rate, increase the reserve
    // ratio for the next two weeks.
    if (annualInterestBips < WildcatMarket(market).annualInterestBips()) {
      TemporaryReserveRatio storage tmp = temporaryExcessReserveRatio[market];

      if (tmp.expiry == 0) {
        tmp.reserveRatioBips = uint128(WildcatMarket(market).reserveRatioBips());

        // Require 90% liquidity coverage for the next 2 weeks
        WildcatMarket(market).setReserveRatioBips(9000);
      }

      tmp.expiry = uint128(block.timestamp + 2 weeks);
    }

    WildcatMarket(market).setAnnualInterestBips(annualInterestBips);
  }

The sanity check is very important this point because there also is no check inside the market contract except the BIP value.

Tools Used

Manual Review

Recommended Mitigation Steps

As discussed with the sponsor, the max and min values can be stored as an immutable in the market contract and checked directly in the WildCatMarket.setAnnualInterestBips() function.

Assessed type

Math