c4-bot-10 commented 1 month ago

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 commented 3 weeks ago

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 commented 3 weeks ago

3docSec changed the severity to QA (Quality Assurance)