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

incorrect staking and unstaking tracking leads to errors

kwakudr

Summary

The FjordStaking contract uses two variables to track staked amounts: totalStaked and newStaked. The inconsistent updating of these variables can lead to potential discrepancies in the total staked amount, affecting reward calculations and overall contract integrity.

Vulnerability Details

if (dr.epoch == 0) {
dr.staked = _amount;
dr.epoch = currentEpoch;
_activeDeposits[msg.sender].add(currentEpoch);
} else {
dr.staked += _amount;
}
​
​
newStaked += _amount;
dr.staked -= _amount;
if (currentEpoch != _epoch) {
totalStaked -= _amount;
userData[msg.sender].totalStaked -= _amount;
} else {
// unstake immediately
newStaked -= _amount;
}

if (latestEpoch > currentEpoch) {
//Time to rollover
currentEpoch = latestEpoch;

if (totalStaked > 0) {
uint256 currentBalance = fjordToken.balanceOf(address(this));
​
// no distribute the rewards to the users coming in the current epoch
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]);
}
} else {
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded];
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
}
​
totalStaked += newStaked;
totalVestedStaked += newVestedStaked;
newStaked = 0;
newVestedStaked = 0;

Let's consider a scenario to illustrate the potential inconsistency:

  1. Current state:

    • totalStaked = 1000

    • newStaked = 0

    • currentEpoch = 10

  2. User A stakes 100 tokens:

    • newStaked becomes 100

    • totalStaked remains 1000

  3. User B unstakes 50 tokens from the current epoch:

    • newStaked becomes 50 (100 - 50)

    • totalStaked remains 1000

  4. User C unstakes 200 tokens from a previous epoch:

    • totalStaked becomes 800 (1000 - 200)

    • newStaked remains 50

  5. Epoch rolls over:

    • totalStaked becomes 850 (800 + 50)

    • newStaked resets to 0

The issue here is that the contract treats staking and unstaking differently based on the epoch, and it updates different variables (totalStaked or newStaked) depending on the situation. The actual total staked amount at any given time is split between totalStaked and newStaked, making it harder to get an accurate total.

If rewards are calculated based on totalStaked before an epoch rollover, they won't account for the newStaked amount.

Impact

errors in tracking of staked amounts, leading to more unreliable reward calculations.

Tools Used
Manual Review

Recommendations

Use only totalStaked and update it consistently in all functions. Remove newStaked entirely.

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.