ElKu - User's request for a second rollover will be ignored if his first request is already minted by mintRollover function

[sherlock-admin]

ElKu

medium

User's request for a second rollover will be ignored if his first request is already minted by mintRollover function

Summary

The Carousel contract keeps tracks of minted rollovers for each epoch by using a state variable called rolloverAccounting. And enlistInRollover function keeps track of each user's rollover details with state variable called rolloverQueue. If there is an entry that already exists for the user, then the new rollover details will be overwritten into the old rolloverQueue. This means that the second request for rollover will be ignored, if the minting is already done on the first request. As, as per rolloverAccounting, that index in rolloverQueue is already processed.

Vulnerability Detail

1. User first opts in for rollover of half of his tokens by calling the enlistInRollover function. Since he is calling this function for the first time, a new entry will be pushed into the rolloverQueue array.
2. Next, before the epoch starts a relayer will call the mintRollovers function.
3. The user's request will be processed, if he has won the previous epoch.
4. Once a queue item is processed, the rolloverAccounting array is updated with the latest index processed by the mintRollovers function.
5. Next time mintRollovers is called, for this specific epochId, the indices less than this saved index will be ignored by this function for minting. This is done here.
6. Now at some point before the epoch actually starts, the user decides to deposit the remaining tokens he has to the upcoming epoch as well. So he calls the enlistInRollover again. This time, since there is an entry already in the name of the user, the rolloverQueue array is updated without pushing a new element into it.
7. Afterwards, a relayer calls the mintRollovers function. But this time, the starting index is past the user's rolloverQueue index. So his rollover request gets ignored.
8. He loses his chance to participate in the epoch and losses his investment opportunity.

Impact

Users request for a second rollover will be ignored if his first request is already minted by mintRollover function.

Code Snippet

The following poc was written,

    function testElku7() public {

        testDepositIntoQueueMultiple();

        // create new epoch
        uint40 _epochBegin = uint40(block.timestamp + 3 days);
        uint40 _epochEnd = uint40(block.timestamp + 4 days);
        uint256 _epochId = 3;
        uint256 _emissions = 100 ether;

        deal(emissionsToken, address(vault), 500 ether, true);
        vault.setEpoch(_epochBegin, _epochEnd, _epochId);
        vault.setEmissions( _epochId, _emissions);

        uint256 prevEpochUserBalance = 10 ether - relayerFee;

        uint256 prevEpoch = 2;
        // enlist in rollover for next epoch
        helperRolloverFromEpoch(prevEpoch, USER,  5 ether - relayerFee);  //vault.enlistInRollover(_epoch, amount, user);
        helperRolloverFromEpoch(prevEpoch, USER2, prevEpochUserBalance);

        // resolve prev epoch
        vm.warp(block.timestamp + 2 days + 1 hours); // warp to one hour after prev epoch end
        vm.startPrank(controller);
        vault.resolveEpoch(prevEpoch);
        vm.stopPrank();

        // simulate prev epoch win
        stdstore
            .target(address(vault))
            .sig("claimTVL(uint256)")
            .with_key(prevEpoch)
            .checked_write(1000 ether);

        console.log("rollover queue length", vault.getRolloverQueueLenght());
        console.log("rollover accounting index for epochId=2 is ", vault.rolloverAccounting(2));
        console.log("rollover accounting index for epochId=3 is ", vault.rolloverAccounting(3));

        console.log("rollover queue format: (assets, receiver, epochId)");
        console.log("");
        (uint a,address b,uint c) = vault.rolloverQueue(0);
        console.log("rollover queue for location 0: ", a, b, c);
        (a,b,c) = vault.rolloverQueue(1);
        console.log("rollover queue for location 1: ", a, b, c);
        console.log("");
        console.log("balance of USER for  epochId 2=", vault.balanceOf(USER, prevEpoch));
        console.log("balance of USER2 for epochId 2=", vault.balanceOf(USER2, prevEpoch));
        console.log("balance of USER for  epochId 3=", vault.balanceOf(USER, _epochId));
        console.log("balance of USER2 for epochId 3=", vault.balanceOf(USER2, _epochId));
        console.log("");

        console.log("calling mintRollovers function... vault.mintRollovers(3, 1000) ");               
        vault.mintRollovers(_epochId, 1000); //  can only mint 6 as queue length is 6
        console.log("rollover accounting index for epochId=2 is ", vault.rolloverAccounting(2));
        console.log("rollover accounting index for epochId=3 is ", vault.rolloverAccounting(3));
        console.log("rollover queue length", vault.getRolloverQueueLenght());
        (a,b,c) = vault.rolloverQueue(0);
        console.log("rollover queue for location 0: ", a, b, c);
        (a,b,c) = vault.rolloverQueue(1);
        console.log("rollover queue for location 1: ", a, b, c);
        console.log("");
        console.log("balance of USER for  epochId 2=", vault.balanceOf(USER, prevEpoch));
        console.log("balance of USER2 for epochId 2=", vault.balanceOf(USER2, prevEpoch));
        console.log("balance of USER for  epochId 3=", vault.balanceOf(USER, _epochId));
        console.log("balance of USER2 for epochId 3=", vault.balanceOf(USER2, _epochId));
        console.log("");

        console.log("calling function vault.enlistInRollover(3, 2 ether-relayerFee, USER)");
        helperRolloverFromEpoch(_epochId, USER,  2 ether - relayerFee);  //vault.enlistInRollover(_epoch, amount, user);
        console.log("calling mintRollovers function... vault.mintRollovers(3, 1000) ");   
        vault.mintRollovers(_epochId, 1000); //  can only mint 6 as queue length is 6
        console.log("rollover accounting index for epochId=2 is ", vault.rolloverAccounting(2));
        console.log("rollover accounting index for epochId=3 is ", vault.rolloverAccounting(3));
        console.log("user balance:",vault.balanceOf(USER, _epochId));
        console.log("rollover queue length", vault.getRolloverQueueLenght());
        (a,b,c) = vault.rolloverQueue(0);
        console.log("rollover queue for location 0: ", a, b, c);
        (a,b,c) = vault.rolloverQueue(1);
        console.log("rollover queue for location 1: ", a, b, c);

        console.log("");
        console.log("balance of USER for  epochId 2=", vault.balanceOf(USER, prevEpoch));
        console.log("balance of USER2 for epochId 2=", vault.balanceOf(USER2, prevEpoch));
        console.log("balance of USER for  epochId 3=", vault.balanceOf(USER, _epochId));
        console.log("balance of USER2 for epochId 3=", vault.balanceOf(USER2, _epochId));

        console.log("");
        console.log("The balance of USER for  epochId 3 didnt increase by 2 ether even though it was added for rollover");

    }

The printed verbose output was:

[PASS] testElku7() (gas: 1955662)
Logs:
  rollover queue length 2
  rollover accounting index for epochId=2 is  0
  rollover accounting index for epochId=3 is  0
  rollover queue format: (assets, receiver, epochId)

  rollover queue for location 0:  4999999998000000000 0xCCA23C05a9Cf7e78830F3fd55b1e8CfCCbc5E50F 2
  rollover queue for location 1:  9999999998000000000 0x0000000000000000000000000000000000012312 2

  balance of USER for  epochId 2= 9999999998000000000
  balance of USER2 for epochId 2= 9999999998000000000
  balance of USER for  epochId 3= 0
  balance of USER2 for epochId 3= 0

  calling mintRollovers function... vault.mintRollovers(3, 1000)
  rollover accounting index for epochId=2 is  0
  rollover accounting index for epochId=3 is  2
  rollover queue length 2
  rollover queue for location 0:  4999999996000000000 0xCCA23C05a9Cf7e78830F3fd55b1e8CfCCbc5E50F 3
  rollover queue for location 1:  9999999996000000000 0x0000000000000000000000000000000000012312 3

  balance of USER for  epochId 2= 5000000000000000000
  balance of USER2 for epochId 2= 0
  balance of USER for  epochId 3= 4999999996000000000
  balance of USER2 for epochId 3= 9999999996000000000

  calling function vault.enlistInRollover(3, 2 ether-relayerFee, USER)
  calling mintRollovers function... vault.mintRollovers(3, 1000)
  rollover accounting index for epochId=2 is  0
  rollover accounting index for epochId=3 is  2
  user balance: 4999999996000000000
  rollover queue length 2
  rollover queue for location 0:  1999999998000000000 0xCCA23C05a9Cf7e78830F3fd55b1e8CfCCbc5E50F 3
  rollover queue for location 1:  9999999996000000000 0x0000000000000000000000000000000000012312 3

  balance of USER for  epochId 2= 5000000000000000000
  balance of USER2 for epochId 2= 0
  balance of USER for  epochId 3= 4999999996000000000
  balance of USER2 for epochId 3= 9999999996000000000

  The balance of USER for  epochId 3 didnt increase by 2 ether even though it was added for rollover

Tool used

Manual Review, VSCode, Foundry

Recommendation

This needs much deeper changes, as there are many problems with the current logic of the code, as I mentioned in my other submissions.

[hrishibhat]

Not a duplicate of #2
User cannot enter the same epoch mint queue twice is a design limitation, hence this issue is not a valid high/medium