Fjord Token Staking

Summary

The FjordPoints contract incorrectly calculates and updates pending points for users before their staked amount is changed, allowing for exploitation of the reward system.

Vulnerability Details

The `updatePendingPoints` modifier, used in `onStaked` and `onUnstaked` functions, updates a user's pending points before their staked amount is modified. This leads to users receiving rewards for tokens they haven't staked yet or for tokens they're about to unstake.

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

Consider this sequence of actions:

1. User starts with 0 tokens staked and 0 pending points.

2. User calls `onStaked(user, 100 tokens)`. - `updatePendingPoints` runs first, calculating pending points as if 100 tokens were already staked. - Then 100 tokens are added to the user's stake.

3. User immediately calls `onUnstaked(user, 100 tokens)`. - `updatePendingPoints` runs again, calculating additional pending points based on 100 tokens. - Then 100 tokens are removed from the user's stake.

4. Result: User has earned pending points for staking 100 tokens over two periods, despite only holding the stake for a minimal time.

This sequence can be repeated to accumulate large amounts of unearned points.

Example calculation:

Assume `pointsPerToken` increased by 0.5 between each action:

1. Initial state: stakedAmount = 0, pendingPoints = 0, lastPointsPerToken = 0

2. After `onStaked`: - pendingPoints = 0 + (100 * (0.5 - 0)) = 50 - stakedAmount = 100

3. After `onUnstaked`: - pendingPoints = 50 + (100 * (1.0 - 0.5)) = 100 - stakedAmount = 0

The user has gained 100 pending points while only staking for a minimal duration.

Impact

The staking reward system can be exploited through rapid staking and unstaking.

Tools Used

Manual review

Recommendations

Move the point calculation logic to the end of the `onStaked` and `onUnstaked` functions.