Claiming rewards from a future not yet existing epoch prevents claiming rewards for those epochs later on

Summary

If a user claims rewards for a future epoch, all epochs are marked as claimed up until that future epoch. This prevents the user from claiming rewards for those epochs later, leading to a loss of rewards.

Vulnerability Detail

Already claimed rewards are tracked in the isEpochClaimed mapping and checked in the RewardsManager.claimRewards function to prevent claiming rewards multiple times. However, the current implementation does not prevent a user from accidentally claiming rewards for a future epoch. This would iterate through all epochs up until the future epoch and mark them all as claimed. This prevents the user from claiming rewards for those epochs later on, leading to a loss of rewards.

Impact

If a user accidentally claims rewards for a future epoch, the rewards are lost and unclaimable.

Code Snippet

contracts/src/RewardsManager.sol#L112

106: function claimRewards(
107:     uint256 tokenId_,
108:     uint256 epochToClaim_
109: ) external override {
110:     if (msg.sender != stakes[tokenId_].owner) revert NotOwnerOfDeposit();
111:
112:     if (isEpochClaimed[tokenId_][epochToClaim_]) revert AlreadyClaimed();
113:
114:     _claimRewards(tokenId_, epochToClaim_);
115: }

contracts/src/RewardsManager.sol#L298

272: function _calculateAndClaimRewards(
273:     uint256 tokenId_,
274:     uint256 epochToClaim_
275: ) internal returns (uint256 rewards_) {
276:
277:     address ajnaPool      = stakes[tokenId_].ajnaPool;
278:     uint256 lastBurnEpoch = stakes[tokenId_].lastInteractionBurnEpoch;
279:     uint256 stakingEpoch  = stakes[tokenId_].stakingEpoch;
280:
281:     uint256[] memory positionIndexes = positionManager.getPositionIndexes(tokenId_);
282:
283:     // iterate through all burn periods to calculate and claim rewards
284:     for (uint256 epoch = lastBurnEpoch; epoch < epochToClaim_; ) {
285:
286:         uint256 nextEpochRewards = _calculateNextEpochRewards(
287:             tokenId_,
288:             epoch,
289:             stakingEpoch,
290:             ajnaPool,
291:             positionIndexes
292:         );
293:
294:         uint256 nextEpoch = epoch + 1;
295:
296:         // update epoch token claim trackers
297:         rewardsClaimed[nextEpoch]           += nextEpochRewards;
298:         isEpochClaimed[tokenId_][nextEpoch] = true;
299:
300:         rewards_ += nextEpochRewards;
301:
302:         unchecked { ++epoch; }
303:     }
304: }

Tool used

Manual Review

Recommendation

Consider adding a check to the RewardsManager.claimRewards function to prevent claiming rewards for future epochs.

sherlock-audit / 2023-01-ajna-judging

berndartmueller - Claiming rewards from a future not yet existing epoch prevents claiming rewards for those epochs later on #122