cccz - enlistInRollover will set the ownerToRollOverQueueIndex incorrectly

[sherlock-admin]

cccz

high

enlistInRollover will set the ownerToRollOverQueueIndex incorrectly

Summary

When ownerToRollOverQueueIndex[_receiver] ! = 0, enlistInRollover will set ownerToRollOverQueueIndex[_receiver] incorrectly.

Vulnerability Detail

In enlistInRollover, if ownerToRollOverQueueIndex[_receiver] ! = 0, the rolloverQueue with the index of the current _receiver is updated, but then the ownerToRollOverQueueIndex[_receiver] is updated to the length of the current rolloverQueue.

    function enlistInRollover(
        uint256 _epochId,
        uint256 _assets,
        address _receiver
    ) public epochIdExists(_epochId) minRequiredDeposit(_assets) {
        // check if sender is approved by owner
        if (
            msg.sender != _receiver &&
            isApprovedForAll(_receiver, msg.sender) == false
        ) revert OwnerDidNotAuthorize(msg.sender, _receiver);
        // check if user has enough balance
        if (balanceOf(_receiver, _epochId) < _assets)
            revert InsufficientBalance();

        // check if user has already queued up a rollover
        if (ownerToRollOverQueueIndex[_receiver] != 0) {
            // if so, update the queue
            uint256 index = getRolloverIndex(_receiver);
            rolloverQueue[index].assets = _assets;
            rolloverQueue[index].epochId = _epochId;
        } else {
            // if not, add to queue
            rolloverQueue.push(
                QueueItem({
                    assets: _assets,
                    receiver: _receiver,
                    epochId: _epochId
                })
            );
        }
        ownerToRollOverQueueIndex[_receiver] = rolloverQueue.length;

Consider the following scenario.
In epoch A, alice calls enlistInRollover, _assets = 10, ownerToRollOverQueueIndex[alice] = 1.
bob calls enlistInRollover, _assets = 1, ownerToRollOverQueueIndex[bob] = 2.
alice calls enlistInRollover again with _assets = 20, at which point ownerToRollOverQueueIndex[alice] is incorrectly set to 2.

In epoch B, since ownerToRollOverQueueIndex[alice] == 2, alice can call withdraw to fetch 19 _assets, while in mintDepositInQueue, since the receiver of rolloverQueue[0] is alice and the balance of alice is not enough, mintDepositInQueue will fail and the bob will not be able to rollover or withdraw the assets.

Impact

It will cause the assets of other users to be locked

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L253-L268

Tool used

Manual Review

Recommendation

Change to

    function enlistInRollover(
        uint256 _epochId,
        uint256 _assets,
        address _receiver
    ) public epochIdExists(_epochId) minRequiredDeposit(_assets) {
        // check if sender is approved by owner
        if (
            msg.sender != _receiver &&
            isApprovedForAll(_receiver, msg.sender) == false
        ) revert OwnerDidNotAuthorize(msg.sender, _receiver);
        // check if user has enough balance
        if (balanceOf(_receiver, _epochId) < _assets)
            revert InsufficientBalance();

        // check if user has already queued up a rollover
        if (ownerToRollOverQueueIndex[_receiver] != 0) {
            // if so, update the queue
            uint256 index = getRolloverIndex(_receiver);
            rolloverQueue[index].assets = _assets;
            rolloverQueue[index].epochId = _epochId;
        } else {
            // if not, add to queue
            rolloverQueue.push(
                QueueItem({
                    assets: _assets,
                    receiver: _receiver,
                    epochId: _epochId
                })
            );
+       ownerToRollOverQueueIndex[_receiver] = rolloverQueue.length;
        }
-       ownerToRollOverQueueIndex[_receiver] = rolloverQueue.length;

Duplicate of #2