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

Improper Distribution of Points Before Updating `PointsPerEpoch`

Summary

The pointsPerEpoch state variable in FjordPoints.sol represents the points distributed per week. The contract owner can update pointsPerEpoch during an epoch; however, points are not properly distributed before updating the pointsPerEpoch.

Vulnerability Details

/**
* @notice Updates the points distributed per epoch.
* @param _points The amount of points to be distributed per epoch.
*/
function setPointsPerEpoch(uint256 _points) external onlyOwner checkDistribution {
if (_points == 0) {
revert();
}
pointsPerEpoch = _points;
}
/**
* @dev Modifier to check and distribute points.
*/
modifier checkDistribution() {
distributePoints();
_;
}
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/main/src/FjordPoints.sol#L184

When the owner calls setPointsPerEpoch() to update the pointsPerEpoch value, the checkDistribution() modifier is triggered to distribute the accumulated points. However, the issue lies in the distribution process, which only accounts for the previous epochs, resulting in incorrect point distribution for the current epoch.

Example Scenarios

Assume an epoch runs from t to t + WEEK, with the current pointsPerEpoch set to PointsPerEpoch. The owner decides to update pointsPerEpoch to updatedPointsPerEpoch at timestamp = t + WEEK - x, where x is a few seconds.

Case 1: updatedPointsPerEpoch > PointsPerEpoch

In this case, the protocol will issue more points than expected for the current epoch, effectively rewarding users more than intended.

Case 2: updatedPointsPerEpoch < PointsPerEpoch

Here, users will receive fewer points than expected, leading to an under-distribution of rewards.

Ideally, the protocol should issue points up to the moment when the owner calls setPointsPerEpoch(). Failing to do so results in incorrect point distribution for the current epoch.

Impact

Incorrect distribution of Points

Tools Used

Manual

Recommendations

If setPointsPerEpoch() is called in the middle of an epoch, the points corresponding to the elapsed time should be distributed, and a new epoch should start from the current block.timestamp.

function setPointsPerEpoch(uint256 _points) external onlyOwner {
if (_points == 0) {
revert();
}
if (totalStaked != 0) {
// Calculate the points for the pending week and the elapsed time in the current epoch
uint256 accumulatedPoints = ((block.timestamp - lastDistribution) *
pointsPerEpoch) / EPOCH_DURATION;
pointsPerToken = pointsPerToken.add(
accumulatedPoints.mul(PRECISION_18).div(totalStaked)
);
totalPoints = totalPoints.add(accumulatedPoints);
// Start a new epoch from the current timestamp with the new pointsPerEpoch
lastDistribution = block.timestamp;
emit PointsDistributed(pointsPerEpoch, pointsPerToken);
}
pointsPerEpoch = _points;
}
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.