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

Incorrect Reward Distribution Across Epochs in FjordStaking Contract

Summary

The _checkEpochRollover function in the FjordStaking contract incorrectly distributes rewards across multiple epochs. This results in the same reward per token being assigned to all epochs between lastEpochRewarded + 1 and currentEpoch - 1 in a case where we have more than one epoch pending. This issue can lead to unfair reward allocation among stakers.

Vulnerability Details

Code Location

The vulnerability is located in the _checkEpochRollover function of the FjordStaking contract:

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L706-L708

function _checkEpochRollover() internal {
uint16 latestEpoch = getEpoch(block.timestamp);
if (latestEpoch > currentEpoch) {
// Time to rollover
currentEpoch = latestEpoch;
if (totalStaked > 0) {
uint256 currentBalance = fjordToken.balanceOf(address(this));
// Calculate the total pending rewards
uint256 pendingRewards = (currentBalance + totalVestedStaked + newVestedStaked)
- totalStaked - newStaked - totalRewards;
uint256 pendingRewardsPerToken = (pendingRewards * PRECISION_18) / totalStaked;
totalRewards += pendingRewards;
// Distribute rewards for each epoch
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
} else {
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded];
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
}
totalStaked += newStaked;
totalVestedStaked += newVestedStaked;
newStaked = 0;
newVestedStaked = 0;
lastEpochRewarded = currentEpoch - 1;
}
}

Issue Description

The loop in the _checkEpochRollover function that updates the rewardPerToken for each epoch between lastEpochRewarded + 1 and currentEpoch - 1 uses the same value for pendingRewardsPerToken. This value is calculated once and added to rewardPerToken[lastEpochRewarded] for each iteration. As a result, all epochs in the range receive the same reward per token, which does not accurately reflect the rewards that should be distributed for each epoch individually.

Example Scenario

Consider the following scenario:

  • lastEpochRewarded is 5.

  • currentEpoch is 10.

  • pendingRewardsPerToken is calculated as 100.

The loop will set rewardPerToken for epochs 6, 7, 8, and 9 to the same value:

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

This results in rewardPerToken[6], rewardPerToken[7], rewardPerToken[8], and rewardPerToken[9] all being set to rewardPerToken[5] + 100.

Impact

The incorrect reward distribution can lead to several issues:

  1. Unfair Reward Allocation: Stakers who participated in different epochs may receive the same rewards, regardless of the actual staking activity and rewards generated in each epoch. This can result in unfair reward allocation among stakers.

  2. Undermined Staking Mechanism: The integrity of the staking mechanism is compromised, as the rewards do not accurately reflect the staking activity in each epoch.

  3. Potential Disputes: Users may dispute the reward distribution, leading to a loss of trust in the staking contract and the overall ecosystem.

Tools Used

  • Manual Code Review

Recommendations

To ensure that rewards are distributed correctly across multiple epochs, the logic in the _checkEpochRollover function should be adjusted to account for the rewards that should be distributed for each epoch individually.

Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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