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

Inaccurate Reward Calculation Due to Delayed Update of totalStaked in Staking Contract

bigsam

Summary

In the staking contract, a user's rewards may be inaccurately calculated as zero when claimed after 1 or 2 epochs of staking. This issue arises because the totalStaked value is not updated before calculating the rewards, resulting in the use of an incorrect total supply. Consequently, the rewards for the previous epoch are missed, and the user is penalized by receiving a lower amount of rewards than they are entitled to.

Vulnerability Details

The `_redeem` function in the staking contract is responsible for accumulating unclaimed rewards for a user and updating their staking data. However, there is a logical flaw in the order of operations within this function, which leads to incorrect reward calculations.

The current implementation of the `_redeem` function is as follows:

/// @notice accumulate unclaimed rewards for the user from last non-zero unredeemed epoch
​
/// This function should run before every tx user does, so state is correctly maintained every time
​
/// Last unredeemed epoch will be the last epoch user staked
​
function \_redeem(address sender) internal {
​
//1. Get user data
​
UserData storage ud = userData\[sender];
​
​
ud.unclaimedRewards +=
​
 
​
@audit>>>> 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;
​
@audit>>>> ud.totalStaked += (deposit.staked + deposit.vestedStaked);
​
}
​
}
​
​

In this function, the rewards are calculated and added to `ud.unclaimedRewards` before the `totalStaked` value is updated:

ud.unclaimedRewards +=
​
@audit>>>> calculateReward(ud.totalStaked, ud.lastClaimedEpoch, currentEpoch - 1);
​
ud.lastClaimedEpoch = currentEpoch - 1;
​

Since `ud.totalStaked` is updated only after the rewards calculation, the function uses an outdated total supply value, leading to a reward calculation of zero for the user. The missed rewards from the previous epoch are effectively skipped, and the user loses out on the rewards they should have received.

Impact

This vulnerability results in users receiving inaccurate rewards, which can be significantly lower than what they are entitled to. The error occurs because the staking data is not correctly updated before the rewards are calculated, causing the contract to overlook the rewards for the previous epoch.

Tools Used

Manual Code Review

Recommendations

1. **Reorder Operations in `_redeem` Function**: The reward calculation should occur after the `totalStaked` value is updated. This change ensures that the correct total supply is used in the calculation. Specifically, move the following lines to the end of the `_redeem` function:

-- ud.unclaimedRewards +=
​
-- calculateReward(ud.totalStaked, ud.lastClaimedEpoch, currentEpoch - 1);
​
-- ud.lastClaimedEpoch = currentEpoch - 1;
​

The corrected `_redeem` function would look like this:

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

Updates

Lead Judging Commences

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