Fjord Token Staking
Fjord
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect reset of `unredeemedEpoch`, resulting in loss of user rewards

bladesec

Summary

The _redeem function resets the unredeemedEpoch wrongly, causing users to lose their rewards.

Vulnerability Details

Whenever a staking operation occurs—whether staking or unstaking—the _redeem function is invoked to calculate the pending reward for the user for the current _epoch period. However, it fails to account for any potential epochs that might have been skipped since the last time it was called. This oversight can result in potential reward losses for the user due to premature resets.

Here's a sample scenario:

  1. At 1 epoch, Jake stakes 100 tokens.

  2. Jake then stakes 100 token when epoch 5. At the same time, he gets 100 reward tokens. And we know his unredeemedEpoch = 10. That is, if Jake performs a staking operation (staking/unstaking) when epoch is greater than 5, the tokens he staked at epoch 5 will be rewarded.

  3. Jake unstake his all tokens with epoch 1. So the unredeemedEpoch will be 0.

  4. After a period of time, for example epoch 10, Jake want to claimReward. However, his reward is still 100 reward tokens with no accrue as unredeemedEpoch is 0.

function _redeem(address sender) internal {
//1. Get user data
UserData storage ud = userData[sender];
​
ud.unclaimedRewards +=
calculateReward(ud.totalStaked, ud.lastClaimedEpoch, currentEpoch - 1);
ud.lastClaimedEpoch = currentEpoch - 1;
​
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

A user may lose their portion of rewards due to improper handling during the unstaking process.

Tools Used

Manual Review

Recommendations

  1. Never reset the unclaimedRewards without fully checking all previous epoch.

  2. Check if the _epoch is equal to currentEpoch.

if (userData[msg.sender].unredeemedEpoch == currentEpoch && _epoch = currentEpoch) {
userData[msg.sender].unredeemedEpoch = 0;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`unredeemedEpoch` is set to zero due to a false if condition in `unstake` and `_unstakeVested` function

Users that try to unstake tokens from an earlier epoch after they staked in the current epoch will have their ` unredeemedEpoch` set to 0, leading to them being unable to access the newly staked tokens. Impact: High – Tokens are lost Likelihood: Medium – It happens every time a user performs the respective sequence of calls. It’s not always but it’s also not a low likelihood scenario. It’s normal usage of the protocol, that doesn’t necessarily require special conditions.

Support

FAQs

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