ck - mintRollovers can fail if too many roll overs don't have adequate shares

[sherlock-admin]

ck

medium

mintRollovers can fail if too many roll overs don't have adequate shares

Summary

mintRollovers can fail if too many rollovers don't have adequate shares

Vulnerability Detail

In the mintRollovers function the number of iterations is determined by length of the rolloverQueue and the index set by rolloverAccounting[_epochId]:

        uint256 length = rolloverQueue.length;
        uint256 index = rolloverAccounting[_epochId];

        // revert if queue is empty or operations are more than queue length
        if (length == 0) revert OverflowQueue();

        if (_operations > length || (index + _operations) > length)
            _operations = length - index;

The value of rolloverAccounting[_epochId] is only updated if there are successful executions.

        uint256 prevIndex = index;
        uint256 executions = 0;

        while ((index - prevIndex) < (_operations)) {
            // only roll over if last epoch is resolved
            if (epochResolved[queue[index].epochId]) {
                uint256 entitledShares = previewWithdraw(
                    queue[index].epochId,
                    queue[index].assets
                );
                // mint only if user won epoch he is rolling over
                if (entitledShares > queue[index].assets) {
                    // skip the rollover for the user if the assets cannot cover the relayer fee instead of revert.
                    if (queue[index].assets < relayerFee) {
                        index++;
                        continue;
                    }
                    // @note we know shares were locked up to this point
                    _burn(
                        queue[index].receiver,
                        queue[index].epochId,
                        queue[index].assets
                    );
                    // transfer emission tokens out of contract otherwise user could not access them as vault shares are burned
                    _burnEmissions(
                        queue[index].receiver,
                        queue[index].epochId,
                        queue[index].assets
                    );
                    // @note emission token is a known token which has no before transfer hooks which makes transfer safer
                    emissionsToken.safeTransfer(
                        queue[index].receiver,
                        previewEmissionsWithdraw(
                            queue[index].epochId,
                            queue[index].assets
                        )
                    );

                    emit Withdraw(
                        msg.sender,
                        queue[index].receiver,
                        queue[index].receiver,
                        _epochId,
                        queue[index].assets,
                        entitledShares
                    );
                    uint256 assetsToMint = queue[index].assets - relayerFee;
                    _mintShares(queue[index].receiver, _epochId, assetsToMint);
                    emit Deposit(
                        msg.sender,
                        queue[index].receiver,
                        _epochId,
                        assetsToMint
                    );
                    rolloverQueue[index].assets = assetsToMint;
                    rolloverQueue[index].epochId = _epochId;
                    // only pay relayer for successful mints
                    executions++;
                }
            }
            index++;
        }

        if (executions > 0) rolloverAccounting[_epochId] = index;

Let's start with the case where rolloverAccounting[_epochId] = 0.
Consider a case where there are no successful executions because all the rollovers that are checked in the while loop don't meet the condition entitledShares > queue[index].assets. That means that rolloverAccounting[_epochId] will not be updated and will remain zero.

If the rolloverQueue continues growing with rollovers that don't meet the share requirements, it will be impossible to rollover the ones that meet the share requirements when they are added to the queue later.

The reason is that whereas the rolloverQueue is growing, the number of _operations that will need to be supplied to the function call will need to be very high to reach the rollovers that are much later in the queue. This accompanied by the fact that rolloverAccounting[_epochId] is still zero because it was never updated means _operations = length - index; will have to be a high number otherwise rollovers that are much later in the queue will not be reached. This will then lead to out of gas errors depending on how large the rolloverQueue grows.

Impact

Rollovers that meet requirements and are added later in the queue will fail.

Code Snippet

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

Tool used

Manual Review

Recommendation

rolloverAccounting[_epochId] = index should be done whether there are successful executions or not.

Duplicate of #172