DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: high
Invalid

For loop inside `_checkEpochRollover` incorectly double the rewards if there are any missed epochs

Impact

  1. Users receive double rewards.

  2. Insolvency occurs as early users claim double rewards, causing later transactions to revert due to insufficient tokens in the contract.

Summary

If an epoch is missed, later when it's rolled users will earn double rewards. Missing an epoch can occur due to the system being new and few users having familiarity with it or any other reason.

Vulnerability Details

_checkEpochRollover is used to advance epochs, calculate rewards for each epoch, and update global values like totalRewards. The function includes a for loop that updates all missed epochs:

for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}

Note that lastEpochRewarded is always set to currentEpoch - 1, meaning that if the function is invoked mid-epoch 6, it will calculate rewards up to epoch 5.

lastEpochRewarded = currentEpoch - 1;

However, this for loop inflates the rewards by adding the pendingRewardsPerToken generated up to now to every missed epoch. If one epoch is missed, pendingRewardsPerToken would be doubled; if two are missed, it would be tripled, and so on.

This issue occurs because the total pendingRewardsPerToken generated from the last triggered epoch is calculated:

uint256 currentBalance = fjordToken.balanceOf(address(this));
uint256 pendingRewards = (currentBalance + totalVestedStaked + newVestedStaked) - totalStaked - newStaked - totalRewards;
uint256 pendingRewardsPerToken = (pendingRewards * PRECISION_18) / totalStaked;
totalRewards += pendingRewards;

But it is then incorrectly added to every missed epoch:

for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}

Example (1 missed epoch):

  1. The last triggered epoch was X (meaning lastEpochRewarded = X-1) with rewardPerToken = 100.

  2. We are now at X+2 with 10 pendingRewardsPerToken generated from X to X+2.

The for loop will perform the following:

Formula:
rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken
Calculation:
rewardPerToken[X] = rewardPerToken[X-1] + pendingRewardsPerToken => 100 + 10 = 110
rewardPerToken[X+1] = rewardPerToken[X] + pendingRewardsPerToken => 110 + 10 = 120

Root Cause

The for loop inside _checkEpochRollover incorrectly adds the entire balance for all missed epochs.

for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}

Tools Used

Manual review.

Recommendations

Instead of adding the total rewards to every epoch, only add them to the current one.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.