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

First current Epoch Staker gains all the Points from the previous Epochs without Staking

robinx33

Summary

When a user stakes tokens, they should typically not be allowed to claim points for that same epoch. This ensures that users earn points based on a full epoch of staking rather than potentially manipulating the system to stake and immediately claim rewards without fully participating in the staking period. The more epochs that have passed without stakers, the first staker would get all the points from them. The user can stake, claim, and unstake and they will still get the points from all the unstaked epochs.

If an epoch passes without any stakers, the system still accumulates points for that epoch. The first user to stake in the current epoch will claim points from the previous epoch, even though they did not stake during that period

Vulnerability Details

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordPoints.sol#L229C4-L249C1

/**
* @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);
}
​

  • The if (totalStaked == 0) {````return;````} checks that if there are no stakes do not calculate, but if another epoch comes and there is a staker, that check will pass and calculate Points, when a second user comes to stake they will be blocked by the if (block.timestamp < lastDistribution + EPOCH_DURATION) { return; } since the first staker updates the lastDistribution that means the previous points will be calculated for the first staker.

PoC

  • Copy the Poc and paste it inside the test/unit/points.t.sol

function testClaimPointsOfThePreviousEpochs() public {
address user1 = address(0x2);
address user2 = address(0x3);
uint256 stakeAmount = 100 ether;
uint256 stakeAmount2 = 100 ether;
​
// No stakers on first epoch
fjordPoints.distributePoints(); // does not matter if this wasn't called
​
​
skip(1 weeks); // 2nd epoch
​
vm.startPrank(staking);
fjordPoints.onStaked(user1, stakeAmount); // user 1 deposit
fjordPoints.onStaked(user2, stakeAmount2); // user2 deposit
vm.stopPrank();
​
​
fjordPoints.distributePoints();
​
vm.prank(user1);
fjordPoints.claimPoints();
​
vm.prank(user2);
fjordPoints.claimPoints();
​
console.log("Points Of user1 Second Epoch: ", fjordPoints.balanceOf(user1));
console.log("Points Of user2 Second Epoch: ", fjordPoints.balanceOf(user2));
​
skip(1 weeks); // 3rd epoch
​
fjordPoints.distributePoints();
​
vm.prank(user1);
fjordPoints.claimPoints();
​
vm.prank(user2);
fjordPoints.claimPoints();
​
console.log("Points Of user1 Third Epoch: ", fjordPoints.balanceOf(user1));
console.log("Points Of user2 Third Epoch: ", fjordPoints.balanceOf(user2));
​
}

Output:

[PASS] testClaimPointsOfThePreviousEpochs() (gas: 296567)
Logs:
Points Of user1 Second Epoch: 100000000000000000000
Points Of user2 Second Epoch: 0
Points Of user1 Third Epoch: 150000000000000000000
Points Of user2 Third Epoch: 50000000000000000000
​

Impact

In Fjord users should not be able to earn points for staking and claiming points within the same epoch.

The first user to stake in an epoch where there were no previous stakers can claim all the points from the previous epochs. This means that if a user stakes immediately after an epoch begins, they can unfairly claim all the rewards from all the last unstaked epochs, even though they had no stake during that period.

Knowledgeable users could exploit this vulnerability by monitoring the staking contract and deliberately staking immediately then unstake after an epoch with no stakers to maximize their rewards unfairly.

I don't know if the users are allowed to claim the last epochs Points but they should at least be shared if that's the case.

Tools Used

Manual review

Recommendations

Modify the reward distribution logic to exclude epochs with a zero total staked amount from being considered in point calculations. Points should only be calculated and distributed for epochs where there was an active stake.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
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.