holyhansss - [HIGH] Carousel.enlistInRollover() incorrectly saves ownerToRollOverQueueIndex, which may result in the loss of user funds.

[sherlock-admin]

holyhansss

high

[HIGH] Carousel.enlistInRollover() incorrectly saves ownerToRollOverQueueIndex, which may result in the loss of user funds.

Summary

enlistInRollover() incorrectly stores the ownerToRollOverQueueIndex variable. This allows a malicious user to delist other users. This can lead to many side effects and even disruptions in service.

Vulnerability Detail

In enlistInRollover(), it saves the user's ownerToRollOverQueueIndex. At this point, for every enlist, it will store it as shown in the code below.

ownerToRollOverQueueIndex[_receiver] = rolloverQueue.length;

Also, the user can update the rolloverQueue via enlistInRollover(). This is problematic because the user's ownerToRollOverQueueIndex[_receiver] will be updated again when the user update rollover states.
Example Scenarios

1. Alice(mal user) calls enlistInRollover()
   a. ownerToRollOverQueueIndex[Alice] = 1
2. Bob calls enlistInRollover()
   a. ownerToRollOverQueueIndex[Bob] = 2
3. Alice call enlistRollover() to update her states.
   a. ownerToRollOverQueueIndex[Alice] = 2
4. Since delist is handled by ownerToRollOverQueueIndex[], Alice can call delistInRollover().
   a. Since Alice has the index of Bob, she can delist Bobs.

Alice can continue to attack normal users using only 1 wei each. In addition, even if it is not an attack, Bob will eventually have the authority to delist others, which can lead to disruptions in the protocol. Also, users will not able to withdraw their deposit.

Impact

The user is unable to withdraw deposits since it is protected with no RollingOver modifier. Also, the protocol may not be used because of critical logical vulnerability.

Code Snippet

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

Tool used

Manual Review

Recommendation

Update ownerToRollOverQueueIndex only when receiver is new user in the rollover queue

// 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;

to

// 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;
        }

Duplicate of #2