Fjord Token Staking

Summary

The point reward calculation in FjordPoints.sol contains a vulnerability where users who stake tokens during a later epoch can receive an unfairly large number of points. This is due to the contract’s handling of point distribution across epochs. Specifically, the contract does not account for when a user begins staking, which leads to over-accounting of points for users who stake after the first epoch.

Vulnerability Details

The contract’s vulnerability stems from the logic in the FjordPoints::distributePoints() and FjordPoints::updatePendingPoints() functions, combined with the way pointsPerToken is updated.

A) Distribution of Points:

• The contract distributes points per epoch using the distributePoints() function. This function calculates the pointsPerToken by dividing the points accrued since the last distribution by the total amount staked.

• If multiple epochs have passed, pointsPerToken accumulates the points from all missed epochs.

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

B) Updating Points for a User:

• When a user stakes tokens, the updatePendingPoints() modifier calculates the user's pending points by multiplying the difference between the current pointsPerToken and the user’s lastPointsPerToken by their staked amount.

• The user’s lastPointsPerToken is then updated to the current pointsPerToken.

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

C) Issue with Late Staking:

• If a user stakes in a later epoch (e.g., epoch 3), the pointsPerToken already reflects points from earlier epochs (e.g., epochs 1 and 2). When updatePendingPoints() is called, the user's pending points are calculated based on pointsPerToken, which includes points from epochs they did not participate in.

• This results in over-accounting, as users receive points for epochs during which they had no staked tokens.

D) Example (ignoring precision adjustments here for simplicity):

Consider below example:

Epoch 1:

• Total Staked: 1000 tokens

• Points Distributed: 100 points

• pointsPerToken = 100 points / 1000 tokens = 0.1 points per token

• Epoch 2:

  • No new stakes or unstakes

  • Points Distributed: 100 points

  • pointsPerToken = 0.1 + 100 points / 1000 tokens = 0.2 points per token

• Epoch 3:

  • New User stakes 500 tokens

  • pointsPerToken = 0.2 + 100 points / 1500 tokens = 0.2667 points per token

  • Since the new user’s lastPointsPerToken was initialized to 0, their pending points would be:

    • pendingPoints = 500 * (0.2667 - 0) = 133.35 points

Here, the user unfairly receives points accrued during epochs 1 and 2 (0.2667 includes these), leading to over-accounting. Specifically, this user could receive 133.35 points in epoch 3, compared to the intended 33.35 points (if only points from epoch 3 were counted).

Impact

This leads to unfair distribution of rewards, where users who stake later receive points for epochs they did not participate in. Further, it leads to more points being minted then what was intended per epoch based on pointsPerEpoch.

Tools Used

Manual review.

Recommendations

Ensure that updatePendingPoints() only considers points accumulated after the user started staking by updating lastPointsPerToken to reflect their entry point into the staking. Please see below example:

Before Mitigation - Setup:

• Epoch 1:

  • Total Staked: 1000 tokens

  • Points Distributed: 100 points

  • pointsPerToken = 100 points / 1000 tokens = 0.1 points per token

• Epoch 2:

  • No new stakes or unstakes

  • Points Distributed: 100 points

  • pointsPerToken = 0.1 + 100 points / 1000 tokens = 0.2 points per token

• Epoch 3:

  • New User stakes 500 tokens

  • pointsPerToken = 0.2 + 100 points / 1500 tokens = 0.2667 points per token

  • The new user’s lastPointsPerToken is initialized to 0.

  • Pending points for the new user = 500 * (0.2667 - 0) = 133.35 points.

Issue:

• The new user unfairly receives 133.35 points, which includes points from epochs 1 and 2, where they were not staking.

After Mitigation - Adjusted Logic:

• Modify the updatePendingPoints() logic to update the user’s lastPointsPerToken before distributing points in distributePoints().

Mitigated Scenario:

Epoch 1 and 2:

• These epochs proceed as before, with pointsPerToken reaching 0.2 by the end of epoch 2.

Epoch 3:

• New User stakes 500 tokens.

• Change:

  • Set the new user’s lastPointsPerToken to the current pointsPerToken (which is 0.2) to reflect their entry point into the staking.

  • This ensures that when the updatePendingPoints() modifier calculates the user's pending points, it uses only the points accrued from epoch 3 onward.

Points Calculation:

• pointsPerToken after distribution in epoch 3:

  • pointsPerToken = 0.2 + 100 points / 1500 tokens = 0.2667 points per token.

• Pending points for the new user:

  • Since lastPointsPerToken was set to 0.2 before the distribution, the calculation will be:

    • pendingPoints = 500 * (0.2667 - 0.2) = 33.35 points.

Impact:

• The new user only receives 33.35 points, which correctly reflects the rewards for epoch 3, without including points from epochs 1 and 2.