volodya - Users can change each other assets value inside RolloverIndex queue

[sherlock-admin]

volodya

high

Users can change each other assets value inside RolloverIndex queue

Summary

Users can have the same index RolloverIndex in the rolloverQueue and change each other assets

Vulnerability Detail

There is a bug in the code. There are many ways how everything can go bad. I only listed two of them. enlistInRollover function not working correctly. We should assign an index only if its don't exist and not all the time

Impact

Code Snippet

    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;

        emit RolloverQueued(_receiver, _assets, _epochId);
    }

Carousel/Carousel.sol#L238

Tool used

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/test/V2/e2e/EndToEndCarouselTest.t.sol#L191

        vm.startPrank(USER);
        //enlist in rollover for next epoch
        Carousel(collateral).enlistInRollover(epochId, 8 ether, USER);
        vm.stopPrank();
        vm.startPrank(USER2);
        //enlist in rollover for next epoch
        Carousel(collateral).enlistInRollover(epochId, 9 ether, USER2);
        vm.stopPrank();
        vm.startPrank(USER);
        //enlist in rollover for next epoch
        Carousel(collateral).enlistInRollover(epochId, 2 ether, USER);
console.log("----");
console.log(Carousel(collateral).getRolloverIndex(USER)); // same index
console.log(Carousel(collateral).getRolloverIndex(USER2)); // same index

Recommendation

    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;

        emit RolloverQueued(_receiver, _assets, _epochId);
    }

Duplicate of #2