Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deposits/borrows deviate from intended implementation #271

Open
c4-bot-8 opened this issue Sep 18, 2024 · 0 comments
Open

Deposits/borrows deviate from intended implementation #271

c4-bot-8 opened this issue Sep 18, 2024 · 0 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_15_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-8
Copy link
Contributor

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

@c4-bot-8 c4-bot-8 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Sep 18, 2024
c4-bot-4 added a commit that referenced this issue Sep 18, 2024
@c4-bot-12 c4-bot-12 added the 🤖_15_group AI based duplicate group recommendation label Sep 18, 2024
howlbot-integration bot added a commit that referenced this issue Sep 20, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_15_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants