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

0xNoodle - Unbounded Growth in Batch Deposit Leading to Potential Denial of Service (DoS) in batchDepositERC721 Function #227

Open
sherlock-admin3 opened this issue Sep 23, 2024 · 0 comments

Comments

@sherlock-admin3
Copy link

sherlock-admin3 commented Sep 23, 2024

0xNoodle

High

Unbounded Growth in Batch Deposit Leading to Potential Denial of Service (DoS) in batchDepositERC721 Function

Summary

https://github.com/morph-l2/morph/blob/main/contracts/contracts/l1/gateways/L1ERC721Gateway.sol#L206

The batchDepositERC721 function of the L1ERC721Gateway contract allows users to deposit multiple ERC721 tokens in a single transaction by providing an array of token IDs (uint256[] calldata _tokenIds).

The function currently does not impose any size limits on the _tokenIds[] array, which opens the contract to potential Denial of Service (DoS) attacks through excessively large batch submissions.

If a malicious or overly large batch is submitted, the transaction could consume excessive gas and exceed the Ethereum block gas limit, causing the function to become inoperable and blocking legitimate users from interacting with the contract.

Vulnerability Details

Unbounded Array Growth:

The _tokenIds[] array in batchDepositERC721 has no inherent size limit. A user can submit a batch with an arbitrarily large number of tokens.

Excessive Gas Consumption:

Each ERC721 token transfer via safeTransferFrom is gas-intensive (roughly 50,000-100,000 gas per transfer). The gas consumption grows linearly with the number of tokens in the batch. Submitting a large batch could lead to gas consumption that exceeds the block gas limit (currently around 30 million on Ethereum mainnet).

DoS Potential:

If the gas limit is exceeded, the transaction will revert. A malicious actor could repeatedly submit large batches, causing frequent reverts, effectively preventing legitimate users from interacting with the contract.

Impact

Denial of Service (DoS):

The contract could become unusable due to excessive gas consumption. If the batch is too large, the transaction will fail, and the contract will not be able to process any deposits or withdrawals until the malicious activity stops.

Gas Consumption:

Legitimate users could experience failed transactions due to other users submitting large batches, leading to wasted gas and potential loss of funds.

System Inoperability:

The entire deposit process could be disrupted if attackers continually submit overly large batches that cause the function to revert due to excessive gas use.

Recommendations

Implement a Maximum Size Limit for the _tokenIds[] Array:
Introduce a maximum size limit for the _tokenIds[] array to prevent unbounded growth. For example:

uint256 MAX_BATCH_SIZE = 250;  // Adjust based on gas estimates

require(_tokenIds.length <= MAX_BATCH_SIZE, "Exceeds max batch size");

Introduce rate limiting to prevent users from spamming the contract with consecutive large batch deposits. This can be done through time-based limitations or a cap on the number of tokens that can be deposited in a specific time window.

Unbounded Batch Size in finalizeBatchWithdrawERC721 Leading to Potential Out-of-Gas Errors and Denial of Service (DoS)

@sherlock-admin3 sherlock-admin3 changed the title Calm Khaki Sealion - Unbounded Growth in Batch Deposit Leading to Potential Denial of Service (DoS) in batchDepositERC721 Function 0xNoodle - Unbounded Growth in Batch Deposit Leading to Potential Denial of Service (DoS) in batchDepositERC721 Function Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant