Deposits/borrows deviate from intended implementation

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/README.md#L243-L249

Vulnerability details

Proof of Concept

First, per the readMe, we can see the below has been stated: https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/README.md#L243-L249

General Questions

Question	Answer
ERC20 used by the protocol	ERC-20s used as underlying assets for markets require no fee on transfer, totalSupply to be not at all close to 2^128, arbitrary mint/burn must not be possible, and name, symbol and decimals must all return valid results (for name and symbol, either bytes32 or a string). Creating markets for rebasing tokens breaks the underlying interest rate model.

This means that the amount of assets that can be borrowed in a market should be up to type(uint128).max.

However whenever a lender calls depositUpTo() to deposit assets, the asset amount is scaled up and added to scaledTotalSupply which is limited to toUint104, see https://github.com/code-423n4/2024-08-wildcat/blob/fe746cc0fbedc4447a981a50e6ba4c95f98b9fe1/src/market/WildcatMarket.sol#L55-L92

  function _depositUpTo(
    uint256 amount
  ) internal virtual nonReentrant returns (uint256 /* actualAmount */) {
    // Get current state
    MarketState memory state = _getUpdatedState();

    if (state.isClosed) revert_DepositToClosedMarket();

    // Reduce amount if it would exceed totalSupply
    amount = MathUtils.min(amount, state.maximumDeposit());

    // Scale the mint amount
    uint104 scaledAmount = state.scaleAmount(amount).toUint104();//@audit
    if (scaledAmount == 0) revert_NullMintAmount();

    // ...snip
    // Increase supply
    state.scaledTotalSupply += scaledAmount;

    // Update stored state
    _writeState(state);

    return amount;
  }

As stated earlier on, this means that the maximum amount of assets that can be borrowed through a market is implicitly limited by type(uint104).max * scaleFactor / 1e27.

When a market is first deployed, its scaleFactor is 1e27, which limits the maximum amount borrowable to type(uint104).max contrary to what's been stated in the docs.

Impact

Borrows can't be more than type(uint104).max assets.

Recommended Mitigation Steps

Increase the precision of scaleFactor to uint128 instead. Alternatively, if this is intended then update the docs.

Assessed type

Context

[laurenceday]

This is also filed in the wardens QA report, and that's the appropriate place for this to be given that this is intended behaviour (it does literally say 'not at all close', although that's arguably not as specific as desired).

See point 6 here: #119

It's worth emphasising that the maximum number for a uint104 is 20,282,409,603,651,670,423,947 trillion, or 20,282,409,603,651 units with 18 decimals. It's extremely unlikely that a memecoin market will pop up on Wildcat that warrants this kind of total supply concern: we're mostly expecting to see stablecoin usage and some major altcoins such as WETH, LDO, CRV etc.

[3docSec]

Similar findings were "sponsor acknowledged" in Wildcat's previous contest on C4

[c4-judge]

3docSec changed the severity to QA (Quality Assurance)

[c4-judge]

3docSec marked the issue as grade-b