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

Incorrect Handling of unredeemedEpoch Allows Double Reward Redemption

atoko

Summary

unredeemedEpoch is not handled correctly

Vulnerability Details

During unstaking the function checks if (userData[msg.sender].unredeemedEpoch == currentEpoch) and only then is when the userData[msg.sender].unredeemedEpoch = 0;, this means that they only set it to 0 when user is doing an instant unstaking but if user was to do the unstake at a later stage this is not set to 0
The incorrect resetting of unredeemedEpoch during the unstake process can lead to a scenario where users are able to redeem rewards from a previous epoch multiple times. This could result in an unfair accumulation of rewards, inflating the user’s unclaimedRewards and leading to an incorrect distribution of rewards within the contract. Additionally, This userData[msg.sender].unredeemedEpoch = currentEpoch; is set while staking which means that every time a user is staking there unredeemed epoch will be set to the current epoch. Lets look at the redeem

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

This is a check for the unredeemedEpoch:

Proof of Concept (PoC)

User Staking:

  • User stakes 100 FJORD tokens during epoch 3.

  • unredeemedEpoch is set to 3 which is the current epoch.

  • User waits and unstakes everything in a Future Epoch (Epoch 9):

User unstakes all the tokens during epoch 9.

if (dr.staked == 0 && dr.vestedStaked == 0) {
// this is true because user is unstaking everyting
}

The unredeemedEpoch is not reset to 0 because the check if (userData[msg.sender].unredeemedEpoch == currentEpoch) fails (since unredeemedEpoch is 3, not 9).
User Redeems Rewards:

When the user calls any function that triggers _redeem, the contract checks that unredeemedEpoch is > 0 aand < currentEpoch rewards from epoch 3 to unclaimedRewards, even though these rewards should not be redeemable again.

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

The unclaimedRewards value may be incorrectly increased by the amount from epoch 3, causing further inaccuracies in reward calculations.

unstakeAll does not handle the `unredeemedEpoch` completely.

Impact

users could redeem multiple times

Tools Used

manual review

Recommendations

Update the unstake functions to reset unredeemedEpoch if the staked amount is reduced to zero, regardless of the epoch:

if (userData[msg.sender].unredeemedEpoch <= 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.

Appeal created

0xbrivan2 Judge
about 1 year ago
atoko Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
atoko Submitter
about 1 year ago
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.