iglyx - Null epoch stakers can't rollover

[sherlock-admin]

iglyx

medium

Null epoch stakers can't rollover

Summary

mintRollovers() checks rollover eligibility with entitledShares > queue[index].assets among other conditions.

This rules out null Vaults depositors who wants and can roll over as they all have entitledShares == queue[index].assets (supposing no precision was lost in previewWithdraw() for them). Such rollovers are fully valid and there is no reason to make it unavailable for such users.

Vulnerability Detail

The check can be loosened even further (say lost users who still have enough funds can be rolled over), but anyway users who has a draw definitely constitute a valid case for rollover. Say rarely it can even be entitledShares == queue[index].assets in a normal, not null, epoch, when user has won (premium equals collateral after the fees and it was a depeg).

As such null epoch users have no preliminary warning of unavailability of rollover for them, it can result in a loss as a user relied on rollover instead of doing manual withdraw-deposit, rollover didn't happened and the user has lost their position, while it might be a hedge for a much bigger one.

Impact

Absence of position when user enlisted in a rollover having ample funds available from a null epoch can result in a loss for them as the position can be a part of bigger picture and functionality unavailability in this case looks to be more a technical oversight than a valid limitation, i.e. users will not expect that rollover from a null epoch be denied.

Code Snippet

mintRollovers() proceeds with a user only when payout is strictly bigger than the initial investment:

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

                // mint only if user won epoch he is rolling over
                if (entitledShares > queue[index].assets) {

For null epoch previewWithdraw() will return value close to _assets as claimTVL == finalTVL:

    function previewWithdraw(uint256 _id, uint256 _assets)
        public
        view
        override(SemiFungibleVault)
        returns (uint256 entitledAmount)
    {
        // entitledAmount amount is derived from the claimTVL and the finalTVL
        // if user deposited 1000 assets and the claimTVL is 50% lower than finalTVL, the user is entitled to 500 assets
        // if user deposited 1000 assets and the claimTVL is 50% higher than finalTVL, the user is entitled to 1500 assets
        entitledAmount = _assets.mulDivDown(claimTVL[_id], finalTVL[_id]);
    }

Tool used

Manual Review

Recommendation

Consider treating null epoch and equality as a valid cases for rollover:

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

-               uint256 entitledShares = previewWithdraw( queue[index].epochId, queue[index].assets);
+               uint256 entitledShares = epochNull[queue[index].epochId] ? queue[index].assets : previewWithdraw( queue[index].epochId, queue[index].assets);
                // mint only if user won epoch he is rolling over
-               if (entitledShares > queue[index].assets) {
+               if (entitledShares >= queue[index].assets) {

Duplicate of #442