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

new users will receive points from previous days in the epoch they did not stake in.

arnie

Summary

new users will receive points from previous days in the epoch they did not stake in.

Vulnerability Details

fjord points are points that users accrue while staking. Below i will explain how a user will be able to claim points from days in the epoch he did not stake in.

function onStaked(address user, uint256 amount)
external
onlyStaking
checkDistribution
updatePendingPoints(user)
{
UserInfo storage userInfo = users[user];
userInfo.stakedAmount = userInfo.stakedAmount.add(amount);
totalStaked = totalStaked.add(amount);
emit Staked(user, amount);
}

the onstaked function is called when a user stakes in order to start the stake of points for a user.
if we look at the modifiers there is 3 onlyStaking which ensure only the staking contract can call the function, checkDistribution which distributes the points that are pending if enough time (1 epoch/ 1 weeks) has passed, and updatePendingPoints which will update the pendingPoints and the lastPointsPerToken of a user.

The problem occurs in teh distributePoints function which is called in onStaked because of the checkDistribution modifier

function distributePoints() public {
if (block.timestamp < lastDistribution + EPOCH_DURATION) {
return;
}
​
if (totalStaked == 0) {
return;
}
​
uint256 weeksPending = (block.timestamp - lastDistribution) / EPOCH_DURATION;
pointsPerToken =
pointsPerToken.add(weeksPending * (pointsPerEpoch.mul(PRECISION_18).div(totalStaked)));
totalPoints = totalPoints.add(pointsPerEpoch * weeksPending);
lastDistribution = lastDistribution + (weeksPending * 1 weeks);
​
emit PointsDistributed(pointsPerEpoch, pointsPerToken);
}

The error is in the if statment.
if (block.timestamp < lastDistribution + EPOCH_DURATION) { return;

Therefore if 1 week has not passed then we do not update the PointsPerToken

This allows a user to stake for 1 block and claim the rewards of 1 week/epoch even though he did not stake the entire week.

for example, there is only 1 block left until epoch is completed, this means that distrubutePoints will not pass the first if statement so we will not update pointsPerToken, the user will receive the old pointsPerToken, then the users points per token will be set at the old value before as shown in snippet below

userInfo.lastPointsPerToken = pointsPerToken;

the next block comes and distributePoints is called and a new pointsPerToken is set,
the user may now claim the points for the entire week of staking/ epoch, even if he did not stake the entire epoch/week.

POC

  1. user stakes when there is 1 block left for epoch to finish

  2. the modifier on onStaked calls for distributePoints since epoch has not fully passed we do not update PointsPerToken

  3. users lastPointsPerToken is set to the old value

  4. the next block and distrubiutePoints is called

  5. the pointsPerToken is updated

  6. the user can now claim the points for the entire week, even though he staked for 1 block.

Impact

A user who staked for less than the epoch will be rewarded with too many points than they should. Making it unfair for the users who did stake the entire epoch

Relevant Lines of Code

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordPoints.sol#L197

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordPoints.sol#L232

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordPoints.sol#L253

Tools Used

manual review

Recommendations

pointsPerToken should be incremented slightly every call to distributePoints to avoid this bug

Updates

Lead Judging Commences

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.