DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: high
Invalid

Incorrect / unfair points reward calculation in `FjordPoints.sol` leading to larger than intended reward payouts

Summary

In FjordPoints.sol, the contract accumulates point distribution across epochs via pointsPerToken. Based on the current mechanism, users can unfairly earn larger rewards than they should when they:

  • Stake tokens, unstake tokens and re-stake tokens after a couple of epochs have passed.

  • Stake a small number of token and accumulate the pointsPerToken across many epochs by not calling FjordPoints::claimPoints. When pointsPerToken has accumulated to a large amount, stake a large number of tokens in that epoch and claim larger rewards then they should.

Vulnerability Details

The updatePendingPoints() function calculates pending rewards based on the difference between the current pointsPerToken and the user’s lastPointsPerToken.

/**
* @dev Modifier to update pending points for a user.
* @param user The address of the user to update points for.
*/
modifier updatePendingPoints(address user) {
UserInfo storage userInfo = users[user];
uint256 owed = userInfo.stakedAmount.mul(pointsPerToken.sub(userInfo.lastPointsPerToken))
.div(PRECISION_18);
userInfo.pendingPoints = userInfo.pendingPoints.add(owed);
userInfo.lastPointsPerToken = pointsPerToken;
_;
}

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

Points are distributed 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.

/**
* @notice Distributes points based on the locked amounts in the staking contract.
*/
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);
}

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

If the lastPointsPerToken is not properly updated when the user re-stakes, the user may receive points for all epochs between their unstake and re-stake, even though they were not participating in staking during that time. Please see below example:

  1. Epoch 1:

    • User stakes 100 tokens.

    • Total Staked: 1000 tokens.

    • Points distributed = 100 points.

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

    • User’s lastPointsPerToken is set to 0.1.

  2. Epoch 2:

    • User unstakes all 100 tokens.

    • Total Staked: 1000 tokens.

    • Points distributed = 100 points.

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

    • User’s lastPointsPerToken is set to 0.2.

  3. Epochs 3-9:

    • Additional points are distributed, but the user is not staking any tokens.

    • Assume pointsPerToken increases to 1.0 by the end of epoch 9.

  4. Epoch 10:

    • User re-stakes 100 tokens.

    • Total Staked: 1000 tokens.

    • pointsPerToken after distribution = 1.1 points per token.

    • The updatePendingPoints() function calculates pending points as:

      • pendingPoints = 100 * (1.1 - 0.2) = 90 points.

Here, the user receives 90 points, which includes rewards for epochs 3-9, even though they were not staked during those epochs.

Similarly, in the case where a user stakes a small number of token and accumulate the pointsPerToken across many epochs by not calling FjordPoints::claimPoints, and increases their stake significatntly when pointsPerToken has accumulated to a large amount before claiming their rewards, they could also unfairly gain extra rewards than intended.

Impact

Incorrect/ unfair accounting of rewards leading to larger than intended payouts of points to users who intentially or unintentially manipulate the system in ways described above. Further, it leads to more points being minted then what was intended per epoch based on pointsPerEpoch.

Tools Used

Manual review.

Recommendations

Consider allocating points on an epoch-by-epoch basis, where users only receive points for epochs they have participated in. This can be done by recording the user's staked amount at the start of each epoch and calculating rewards based on that specific epoch.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.