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

Precision Loss in `FjordPoints::distributePoints` Function Can Lead to Incorrect Points Distribution

4gontuk

Summary:

The distributePoints function in the FjordPoints contract suffers from a precision loss issue due to the order of operations in its calculations. This can result in an incorrect distribution of points, potentially disadvantaging users.

Vulnerability Details:

The FjordPoints contract is designed to distribute points based on the amount of tokens staked by users. The distributePoints function is responsible for calculating and distributing these points periodically.

The function calculates the pointsPerToken by multiplying the number of weeks pending (weeksPending) by the points to be distributed per epoch (pointsPerEpoch), and then dividing by the total amount of tokens staked (totalStaked). However, the current implementation performs the division before the multiplication, which can lead to precision loss due to Solidity's integer division rounding down.

The problematic code is as follows:

File: FjordPoints.sol
242: pointsPerToken =
243: pointsPerToken.add(weeksPending * (pointsPerEpoch.mul(PRECISION_18).div(totalStaked)));

In this line, pointsPerEpoch is multiplied by PRECISION_18 and then divided by totalStaked, which can result in a loss of precision. This precision loss can accumulate over time, leading to an incorrect calculation of pointsPerToken.

Impact

The precision loss in the distributePoints function can lead to an incorrect distribution of points among users. This can result in users receiving fewer points than they are entitled to, potentially disadvantaging them. Over time, this can lead to significant discrepancies in the distribution of points, undermining the fairness and accuracy of the points distribution mechanism.

Proof of Concept

  1. Assume weeksPending is 2, pointsPerEpoch is 100 ether, PRECISION_18 is 1e18, and totalStaked is 1000 ether.

  2. The current implementation calculates pointsPerToken as:
    pointsPerToken = pointsPerToken.add(2 * (100 ether * 1e18 / 1000 ether))
    This results in pointsPerToken being incremented by 2 * 1e20, which is 2e20.

  3. The correct calculation should be:
    pointsPerToken = pointsPerToken.add((2 * 100 ether * 1e18) / 1000 ether)
    This results in pointsPerToken being incremented by 2e21 / 1000, which is 2e18.

Tools Used

Manual review

Recommendation

To fix the precision loss issue, the multiplication should be performed before the division. The corrected code is as follows:

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

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Division before multiplication

Support

FAQs

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