minhtrng - Delisting a processed rollover item causes skip of unprocessed one

[sherlock-admin]

minhtrng

medium

Delisting a processed rollover item causes skip of unprocessed one

Summary

If a rollover item is delisted after being processed, the last item in the queue will take its place and not be processed for this epoch

Vulnerability Detail

The function Carousel.delistInRollover can perform a swap of the item to be deleted with the last item in the queue:

} else {
    // overwrite the item to be removed with the last item in the queue
    rolloverQueue[index] = rolloverQueue[length - 1];
    // remove the last item in the queue
    rolloverQueue.pop();
    // update the index of prev last user ( mapping index is allways array index + 1)
    ownerToRollOverQueueIndex[rolloverQueue[index].receiver] =
        index +
        1;
    // remove receiver from index mapping
    delete ownerToRollOverQueueIndex[_owner];
}

If the rolloverQueue is only partially processed and the value at index already has been rolled over, then this will cause the last item to take its place and not be taken into account when continuing the rollover.

Impact

Harm to user

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/ae7f210d8fbf21b9abf09ef30edfa548f7ae1aef/Earthquake/src/v2/Carousel/Carousel.sol#L292-L303

Tool used

Manual Review

Recommendation

Fill the place with a dummy item (e.g. assetValue == 0) and perform the swap and pop when encountering it during the actual rollover function.

Duplicate of #72