Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

0xmuxyz - A malicious user can create and enlist too many deposit queues at row cost, which lead to reverting the transaction because of reaching the block gas limit #342

Closed
sherlock-admin opened this issue Mar 27, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 27, 2023

0xmuxyz

high

A malicious user can create and enlist too many deposit queues at row cost, which lead to reverting the transaction because of reaching the block gas limit

Summary

A malicious user can create and enlist too many deposit queues at row cost, which lead to reverting the transaction of the Carousel#mintDepositInQueue(). Because its transaction will reach the block gas limit.

In addition to that, any users can enlist the deposit queues as much as the users want by calling the Carousel#_deposit() by calling the Carousel#deposit() or the Carousel#depositETH(). And there is no minimum deposit amount within the Carousel#deposit() or the Carousel#depositETH().
These things above allows a malicious user to be able to enlist too many queues at row cost.

Vulnerability Detail

Within the Carousel, the depositQueue array would be defined like this:
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L27

    QueueItem[] public depositQueue;

When the Carousel#deposit() or the Carousel#depositETH() would be called,
the Carousel#_deposit() would be called like this:
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L98
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L117

Within the Carousel#_deposit(), a deposit queue would be enlisted by pushing it to the depositQueue array like this:
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L495-L497

    function _deposit(
        uint256 _id,
        uint256 _assets,
        address _receiver
    ) internal {
        ...
        } else {
            depositQueue.push(  /// @audit
                QueueItem({assets: _assets, receiver: _receiver, epochId: _id})
            );
           ...

After that, within the Carousel#mintDepositInQueue(), all of deposit queues enlisted in the the depositQueue array would be executed by using the loop like this:
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L331

    function mintDepositInQueue(uint256 _epochId, uint256 _operations)
        external
        epochIdExists(_epochId)
        epochHasNotStarted(_epochId)
        nonReentrant
    {
        // make sure there is already a new epoch set
        // epoch has not started
        QueueItem[] memory queue = depositQueue;
        uint256 length = depositQueue.length;

       ...
          // queue is executed from the tail to the head
        // get last index of queue
        uint256 i = length - 1;
        while ((length - _operations) <= i) { /// @audit
            // this loop impelements FILO (first in last out) stack to reduce gas cost and improve code readability
            // changing it to FIFO (first in first out) would require more code changes and would be more expensive
            _mintShares(
                queue[i].receiver,
                _epochId,
                queue[i].assets - relayerFee
            );
            ...

            depositQueue.pop();
            if (i == 0) break;
            unchecked {
                i--;
            }
        }

Within the Carousel#mintDepositInQueue() above, the loop** would be used for executing all of deposit queues enlisted in the the depositQueue array.

In addition to that, any users can enlist the deposit queues as much as the users want by calling the Carousel#_deposit() by calling the Carousel#deposit() or the Carousel#depositETH(). And there is no minimum deposit amount within the Carousel#deposit() or the Carousel#depositETH().
These things above allows a malicious user to be able to enlist too many queues at row cost.

example).
Assuming that a malicious user deposit just 0.001 (_assets) of token into the Vault to create a deposit queue by calling the Carousel#deposit(), the malicious user can 1000 deposit queues by depositing just 1 (_asset) of tokens into the Vault.

If too many queues would be enlisted by a malicious user, too many processing will be executed via the loop within the Carousel#mintDepositInQueue().
As a result, the transaction of the Carousel#mintDepositInQueue() will be reverted in the loop. Because its transaction will reach the block gas limit.

Impact

A malicious user can create and enlist too many deposit queues at row cost, which lead to reverting the transaction of the Carousel#mintDepositInQueue(). Because its transaction will reach the block gas limit.

Code Snippet

Tool used

Manual Review

Recommendation

  • Consider setting the limit of the deposit queues to be executed when the Carousel#mintDepositInQueue() would be called.
    Or,
  • Consider setting the minimum deposit amount to avoid that a malicious user can create too many deposit queues at low cost.

Duplicate of #174

@github-actions github-actions bot closed this as completed Apr 3, 2023
@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 3, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant