Fjord Token Staking

Summary

rewardPerToken sometimes does not update correctly, so the user loses his reward.

Vulnerability Details

background:

1. The protocol adopts the rewardPerToken model. Before the total reward and total pledge amount change, rewardPerToken and the settlement user’s reward income will be updated first. rewardPerToken represents the cumulative reward of each token in each epoch.

2. When updating rewardPerToken, it will only be updated if totalStaked>0, and will not be updated if totalStaked==0.

3. When settling user rewards, if the user's pledge meets the conditions (ud.unredeemedEpoch > 0 && ud.unredeemedEpoch < currentEpoch), the rewards should also be settled.

So assume the following:

1. The user pledges 100 in the first epoch.

2. The user claims it rewards in the third epoch. At this time, 2 epochs have passed, so the user should receive the reward.

   But in fact, since this is the first time the _checkEpochRollover function is called after staking, totalStaked is still 0, so rewardPerToken is still 0.

   if (latestEpoch > currentEpoch) {

   //Time to rollover

   currentEpoch = latestEpoch;

   ​

   if (totalStaked > 0) {

   ···

   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]);

   }

   }

   Then execute the _redeem function. Since rewardPerToken is 0, the calculated result of the calculateReward function must be 0.
   In fact, users pledged in epochs 1~3, but did not receive any rewards.

   if (ud.unredeemedEpoch > 0 && ud.unredeemedEpoch < currentEpoch) {

   // 2. Calculate rewards for all deposits since last redeemed, there will be only 1 pending unredeemed epoch

   DepositReceipt memory deposit = deposits[sender][ud.unredeemedEpoch];

   ​

   // 3. Update last redeemed and pending rewards

   ud.unclaimedRewards += calculateReward(

   deposit.staked + deposit.vestedStaked, ud.unredeemedEpoch, currentEpoch - 1

   );

   ​

   ud.unredeemedEpoch = 0;

   ud.totalStaked += (deposit.staked + deposit.vestedStaked);

   }

Impact

Users may lose part of their rewards.
So the impact is high
Possibility: Since only the first user to call the _checkEpochRollover function will suffer the loss, the possibility is Med/Low.
The final rating I think is H/M.

Tools Used

manual

Recommendations

It is recommended to add newStaked to judge together.