Fjord Token Staking

Summary

The issue arises from improper reward accounting when an epoch is prematurely rolled over. Specifically, if a user triggers an epoch rollover before rewards are correctly accounted for, the rewardPerToken for that epoch is set lower than expected. When the rewardAdmin subsequently adds rewards, these are incorrectly carried over to the next epoch, resulting in inflated rewards for stakers from the previous epoch. This discrepancy allows stakers to accrue more rewards than they are entitled to, creating financial imbalances and potentially undermining the fairness and integrity of the protocol.

Vulnerability Details

The _checkEpochRollover function is a critical component of the FjordStaking contract, responsible for managing epoch transitions and updating the rewardPerToken for epochs. It ensures that rewards are accurately distributed based on the staked amounts and the rewards available in the contract. This function is designed to be called once per epoch, typically triggered by the addReward function.

When the rewardAdmin calls the addReward function, it transfers rewards to the contract and subsequently calls _checkEpochRollover. This updates the epoch and calculates the rewardPerToken for the previous epoch based on the newly added rewards. The design ensures that _checkEpochRollover is only executed once per epoch, preventing redundant updates.

The vulnerability arises when a user, either intentionally or unintentionally, triggers an epoch rollover by calling a function that includes _checkEpochRollover in its modifiers before the rewardAdmin has added rewards. This premature rollover sets the rewardPerToken for the previous epoch inaccurately, as the rewards have not yet been accounted for. Someone can also intentionally frontrun the rewardAdmin, when ever he calls the addReward function, the user can front run the rewardAdmin and stake a small amount causing the epoch to roll over and for the rewardPerToken to be set without accounting for any new rewards.

POC

Add the following poc to the addReward.t.sol file

note: Also add the these functions to the FjordStaking.sol contract and set the FjordStaking::calculateReward function as public for this test to work properly. And mint the user (bob) some tokens at the FjordStakingBase::setUp function.

Impact

Unaccounted Rewards: When epoch is rolled over by another user, no rewards will have been transffered to the contract and when the rewardAdmin eventually calls addReward function in the epoch where the _checkEpochRollover is called once, the rewards will be transferred to the contract but remain unaccounted for, as _checkEpochRollover has already been executed for the current epoch.

Cumulative Reward Discrepancy: In the subsequent epoch, when addReward is called on time, both the new rewards and the previously unaccounted rewards are included in the rewardPerToken calculation for the last epoch. This results in an inflated rewardPerToken for that epoch.

Tools Used

Manual Review

Recommendations

There could be a few fixes to this issue:
1- Implement logic to dynamically adjust the rewardPerToken in the next epoch if it is detected that the previous epoch's rewards were not fully accounted for.
2- Consider decoupling the epoch rollover logic from user actions. Instead of relying on user-triggered functions to call _checkEpochRollover, implement a dedicated function or mechanism that handles epoch transitions independently, or ristrict its access from users.