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

Stakers don't receive the reward they are entitled to for staking when they unstake before the rewards are added

0xbrivan2

Summary

The Fjord Staking contract does not allocate rewards to users who unstake before rewards are added, resulting in those users not receiving their entitled rewards for valid staking periods.

Vulnerability Details

In the Fjord Staking contract, users stake FJORD tokens to earn rewards, which are distributed proportionally based on the amount staked. Rewards are allocated when the admin adds them to the contract:

function addReward(uint256 _amount) external onlyRewardAdmin {
if (_amount == 0) revert InvalidAmount();
uint16 previousEpoch = currentEpoch;
>>> fjordToken.safeTransferFrom(msg.sender, address(this), _amount);
>>> _checkEpochRollover();
emit RewardAdded(previousEpoch, msg.sender, _amount);
}
​
function _checkEpochRollover() internal {
// --SNIP
if (totalStaked > 0) {
uint256 currentBalance = fjordToken.balanceOf(address(this));
uint256 pendingRewards = (currentBalance + totalVestedStaked + newVestedStaked) - totalStaked - newStaked - totalRewards;
uint256 pendingRewardsPerToken = (pendingRewards * PRECISION_18) / totalStaked;
totalRewards += pendingRewards;
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
}
// --SNIP
}

All stakers should receive a proportional share of the rewards added by the admin. However, the issue is that when users stake during valid epochs and then unstake before the admin adds the rewards. The contract lacks a mechanism to allocate rewards for those stakers, even though they are entitled to a share for the valid epochs they participated in. The problem occurs because, upon unstaking, the totalStaked value of the user is reduced:

function unstake(uint16 _epoch, uint256 _amount) external checkEpochRollover redeemPendingRewards returns (uint256 total){
// --SNIP
dr.staked -= _amount;
if (currentEpoch != _epoch) {
totalStaked -= _amount;
>>> userData[msg.sender].totalStaked -= _amount;
}
}

And when rewards are subsequently added, they are calculated based on the user's totalStaked (that was already reduced by the unstake operation):

function _redeem(address sender) internal {
UserData storage ud = userData[sender];
>>> ud.unclaimedRewards += calculateReward(ud.totalStaked, ud.lastClaimedEpoch, currentEpoch - 1);
​
// --SNIP
}

As a result, the contract does not account for rewards owed to users who staked during valid epochs but unstaked before rewards were distributed.

Proof Of Concept

Consider the following test case:

  1. Alice stakes 1 ether during epoch 1.

  2. After 6 weeks, the admin adds rewards for each epoch.

  3. Alice claims her rewards for the past 6 weeks, successfully receiving 6 ether.

  4. An additional 4 weeks pass without the admin adding rewards. During this time, Alice continues staking.

  5. Alice unstakes her tokens, having not received rewards for epochs 7 to 11.

  6. When the admin eventually adds rewards, Alice is unable to claim her share corresponding to epochs 7 to 11.

Copy and paste the following test case into test/unit/stake.t.sol:

function test_unstake_without_earning_rewards() public {
// Alice stakes 1 ether on epoch 1
vm.startPrank(alice);
fjordStaking.stake(1 ether);
​
// The admin added rewards for the next 6 weeks
_addRewardAndEpochRollover(1 ether, 6);
// Alice successfully gets her rewards of 6 ether because of 6 weeks staking
vm.startPrank(alice);
fjordStaking.claimReward(false);
(,uint256 unclaimedRewards,,) = fjordStaking.userData(address(alice));
assertEq(6 ether, unclaimedRewards);
​
// 4 weeks are passed with no rewards added by the admin. Alice was still staking in this period
skip(4 weeks);
​
fjordStaking.completeClaimRequest(); // completing the previous claim rewards request
​
// Alice decides to unstake
fjordStaking.unstakeAll();
​
// The admin later adds rewards. Alice SHOULD have rewards proportion that corresponds to her 4 weeks staking
vm.startPrank(minter);
fjordStaking.addReward(30 ether);
​
// Alice won't have portion of the rewards
vm.startPrank(alice);
vm.expectRevert(FjordStaking.NothingToClaim.selector);
fjordStaking.claimReward(false);
}

Impact

Users who stake during valid epochs and then unstake before rewards are added do not receive their entitled rewards for the epochs in which they participated. This results in an unfair distribution of rewards and breaks a core invariant that stakers should be rewarded for epochs they stake in.

Tools Used

Manual Review

Recommendations

Consider implementing a mechanism to track and allocate rewards for users who unstake before rewards are added. This would ensure that all users receive the rewards they are entitled to for their staking period.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Appeal created

0xbrivan2 Submitter
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
0xbrivan2 Submitter
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!