The Wildcat Protocol - joaovwfreire's results

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol#L474-L481

Vulnerability details

Impact

Even though the protocol in being built with the intent of providing undercollateralized loans, there are no constraints that limit a user if it wants to use it as an overcollateralized loan platform. The setAnnualInterestBips function at the WildcatMarketController contract will set the reserve ratio to 90% if the market borrower or the market controller decides to lower the annual interest regardless of the original reserveRatio value. [1].

    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);
      }

However, the function does not account for the fact that the reserve ratio might be bigger than 90%, meaning a borrower can maliciously lure lenders into an overcollateralized pool with very high annual interest rates, then set it to very small amounts per year and withdraw more collateral than what was originally possible.[2]

Proof of Concept

The following test demonstrates how a Market with 95% reserve ratio can lower it's annual interest rates while also decreasing it's reserve ratio to 90%:

  function test_LowerReserveRatioOnLessAnnualInterest() external asAccount(parameters.controller) {

    // 1. Set the reserve ratio to 95%

    uint16 initialReserveRatioBips = 9500; // 95%

    market.setReserveRatioBips(initialReserveRatioBips);

    assertEq(market.reserveRatioBips(), initialReserveRatioBips, "Reserve ratio should be 95%");

  

    // 2. Lower the annual interest rate

    uint16 loweredAnnualInterestBips = 1; // 0.01% for example

    vm.startPrank(borrower);

    controller.setAnnualInterestBips(address(market), loweredAnnualInterestBips);

    assertEq(market.annualInterestBips(), loweredAnnualInterestBips, "Annual interest rate should be lower");

  

    // 3. Check if the reserve ratio is now 90%

    uint16 expectedReserveRatioBips = 9000; // 90%

    assertEq(market.reserveRatioBips(), expectedReserveRatioBips, "Reserve ratio should be 90% after lowering annual rate");

    console.log("* * Finished * *");

  }

Kindly place this test at WildcatMarketController.t.sol line 295 and run it with:

forge test --match-test "test_LowerReserveRatioOnLessAnnualInterest" -vvv

Tools Used

Manual review

Recommended Mitigation Steps

Check if the current reserve ratio is bigger than 90% before setting it to 90%. a. If it is, then increase by a different factor AND check if it doesn't overflow the BIP maximum value; b. If it isn't, then it should be okay to maintain the current logic.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The function enforceParameterConstraints at the WildcatMarketController contract ensures that annualInterestBips, delinquencyFeeBips, withdrawalBatchDuration, reserveRatioBips and delinquencyGracePeriod are within the allowed ranges and that namePrefix and symbolPrefix are not null. This function is only called during market deployment, at the deployMarket method. [1].

One part of the issue lies on the WildcatMarketConfig contract. This contract has methods that allows the controller and the borrower of a market to change it's parameters, such as updating annualInterestBips and reserveRatioBips without checking the bounds delimited by enforceParameterConstraints. So controllers are able to setReserveRatioBips and to setAnnualInterestBips arbitrarily.[2]

The other part of the issue lies on the WildcatMarketController contract. The setAnnualInterestBips and the resetReserveRatio functions don't check the bounds originally delimited by enforceParameterConstraints during a market's deployment. [3]

Proof of Concept

enforceParameterConstraints checks if annualInterestBips is within bounds determined by maximum and minimum values:

function enforceParameterConstraints(
    string memory namePrefix,
    string memory symbolPrefix,
    uint16 annualInterestBips,
    uint16 delinquencyFeeBips,
    uint32 withdrawalBatchDuration,
    uint16 reserveRatioBips,
    uint32 delinquencyGracePeriod
  ) internal view virtual {
  ...
assertValueInRange(
      annualInterestBips,
      MinimumAnnualInterestBips,
      MaximumAnnualInterestBips,
      AnnualInterestBipsOutOfBounds.selector
    );
...
}

The following functions do not fully substitute the checks made at enforceParameterConstraints: 2:

function setAnnualInterestBips(uint16 _annualInterestBips) public onlyController nonReentrant {
    MarketState memory state = _getUpdatedState();

    if (_annualInterestBips > BIP) {
      revert InterestRateTooHigh();
    }

    state.annualInterestBips = _annualInterestBips;
    _writeState(state);
    emit AnnualInterestBipsUpdated(_annualInterestBips);
  }

3:

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);
  }

Tools Used

Manual review

Recommended Mitigation Steps

Include calls to enforceParameterConstraints internal logic to make sure the proposed annualInterestBips are within the intended bounds. One good option is to call assertValueInRange inside the functions that change market configs.

Assessed type

Context