Reward may be locked forever if user doesn't claim reward for a very long time such that too many epochs have been passed

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/ExtraRewardsDistributor.sol#L233-L240
https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraLocker.sol#L334-L337

Vulnerability details

Impact

Reward may be locked forever if user doesn't claim reward for a very long time such that too many epochs have been passed. The platform then forced to reimburse reward to the user that got their reward locked. Causing huge economics loss.

Proof of Concept

Can be done by reverse engineering from the affected code

        for (uint256 i = epochIndex; i < tokenEpochs; i++) {
            //only claimable after rewards are "locked in"
            if (rewardEpochs[_token][i] < latestEpoch) {
                claimableTokens += _claimableRewards(_account, _token, rewardEpochs[_token][i]);
                //return index user claims should be set to
                epochIndex = i + 1;
            }
        }

From this line you will see a loop from epochIndex to tokenEpochs which loop tokenEpochs - epochIndex times.
If tokenEpochs - epochIndex value goes high, it will consume too much gas which go beyond the limit of the chain and cause the transaction to be always failed. As a result, reward may be locked forever.

        uint256 latestEpoch = auraLocker.epochCount() - 1;
        // e.g. tokenEpochs = 31, 21
        uint256 tokenEpochs = rewardEpochs[_token].length;

        // e.g. epochIndex = 0
        uint256 epochIndex = userClaims[_token][_account];
        // e.g. epochIndex = 27 > 0 ? 27 : 0 = 27
        epochIndex = _startIndex > epochIndex ? _startIndex : epochIndex;

• epochIndex is the maximum of _startIndex and latest index of rewardEpochs that user has claim the reward
• tokenEpochs is the number of epochs that has reward, can be added through addRewardToEpoch function up to latest epoch count of auraLocker
• latestEpoch is epoch count of auraLocker

If you specified too high _startIndex, the reward may be skipped and these skipped reward are lost forever as the _getReward function set latest epoch that user has claim to the lastest index of rewardEpochs that can be claimed.

the aura locker epoch can be added by using checkpointEpoch function which will automatically add epochs up to current timestamp. Imagine today is 100 years from latest checkpoint and rewardsDuration is 1 day, the total of around 36500 epochs needed to be pushed into the array in single transaction which always failed due to gasLimit. The code that responsible for pushing new epochs below (in AuraLocker file)

            while (epochs[epochs.length - 1].date != currentEpoch) {
                uint256 nextEpochDate = uint256(epochs[epochs.length - 1].date).add(rewardsDuration);
                epochs.push(Epoch({ supply: 0, date: uint32(nextEpochDate) }));
            }

Even if these line are passed because the nature that checkpointEpoch is likely to be called daily and reward are added daily. if user doesn't claim the reward for 100 years, rewardEpochs[_token].length = 36500 where epochIndex = 0. Which cause an impossible loop that run 36500 times. In this case this transaction will always be failed due to gas limit. In the worst case, If this problem cause staking fund to be frozen, the only way is to trash the reward and use emergencyWithdraw to withdraw staked fund.

From above statement, we can proof that there exists a case that user reward may be locked forever due to looping too many times causing gas to be used beyond the limit thus transaction always failed.

Tools Used

Can be reverse engineering using the help of IDE.

Recommended Mitigation Steps

User should be able to supply endEpochIndex to the claim reward functions. And only calculate reward from startIndex to min(auraLocker.epochCount() - 1, endEpochIndex). And also add support for partial reward claiming.

[0xMaharishi]

Valid report although given these are reward tokens and the max amount of entries is one per week, it would take some years for this to run over gas limit, during which time the contract could easily be changed

[0xMaharishi]

code-423n4/2022-05-aura#6

[dmvt]

I'm downgrading this to medium severity. It is unreasonable to expect contracts to be future proof to the tune of a hundred years or more, but if the frequency had been unreasonably fast this issue could have kicked in.