Borrower can reduce the maxTotalSupply below the current totalSupply()

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarketConfig.sol#L101-L111

Vulnerability details

Vulnerability Details

According to the documentation; the borrower should not be able to set the maxTotalSupply below the outstanding supply of market tokens

"As a borrower, you are able to adjust the capacity up to whatever amount you wish, or down to the market's current outstanding supply of market tokens"

This is in order to maintain stability in the market though it is not enforced in setMaxTotalSupply() so borrowers are free to set it to what they want as shown in the test below.

POC

Add the test function below to WildcatMarket.t.sol and run:

Reduce capacity

  function test_POC_4() external asAccount(borrower) {

    // scaleFactor is 1 so normalized amount also 100_000
    _deposit(alice, 100_000);
    assertEq(market.totalSupply(), 100_000);

    uint256 currentMaxTotalSupply = market.maxTotalSupply();
    assertEq(currentMaxTotalSupply, 20282409603651670423947251286015);

    market.setMaxTotalSupply(100);
    assertEq(market.maxTotalSupply(), 100);

  }

Tools Used

Manual Review
Foundry Testing

Recommendations

Add a check in setMaxTotalSupply to ensure the new value is not less than totalSupply():

function setMaxTotalSupply(
  uint256 _maxTotalSupply
) external onlyBorrower nonReentrant sphereXGuardExternal {

  MarketState memory state = _getUpdatedState();
  
  // Revert if the market is closed
  if (state.isClosed) revert_CapacityChangeOnClosedMarket();

+ if (_maxTotalSupply < state.totalSupply()) revert_NewCapLessThanCurrentSupply();

  // Call the hook for max total supply update
  hooks.onSetMaxTotalSupply(_maxTotalSupply, state);
  
  // Update the state's max total supply
  state.maxTotalSupply = _maxTotalSupply.toUint128();

  // Persist the state change
  _writeState(state);

  // Emit an event indicating the total supply cap has been updated
  emit_MaxTotalSupplyUpdated(_maxTotalSupply);
}

Assessed type

Invalid Validation

[d1ll0n]

This is a documentation / QA issue not a medium - maxTotalSupply is only the cap at which deposits stop being accepted, so reducing it below the current supply only prevents further deposits. In fact, there are cases where you would specifically want it to be less, such as if a borrower wants to set a much lower cap for deposits so that if lenders withdraw below the new limit, deposits are only allowed back up to it.

ex:

• supply = 100
• borrower only wants to keep ~60, sets maxTotalSupply = 60
• lenders withdraw 50
• deposits capped at another 10

This prevents scenarios where the borrower needs to keep pushing it downward

[c4-judge]

3docSec changed the severity to QA (Quality Assurance)

[c4-judge]

3docSec marked the issue as grade-b