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

distributePoints will reward the first depositor for all the points from the first epoch

pyro

Impact

The first depositor receives all the rewards from the first epoch, even if they are not the only depositor or even if they haven't staked for it.

  1. Depositor games the system.

  2. The first depositor earns rewards from the first epoch even if they have staked in the second.

Summary

distributePoints will reward the first depositor with all the points from the first epoch due to this if check:

if (totalStaked == 0) {
return;
}

Vulnerability Details

FjordPoints is called through FjordStaking and used as another form of rewards. To simplify the report, we avoid the mechanisms of the call from FjordStaking. The only important part to note is that on each stake/unstake, FjordStaking calls onStaked or onUnstaked in FjordPoints.

function stake(...) external checkEpochRollover redeemPendingRewards {
// ... extra code
fjordToken.safeTransferFrom(msg.sender, address(this), _amount);
points.onStaked(msg.sender, _amount);
}
​
function unstake(...) external checkEpochRollover redeemPendingRewards returns (uint256 total) {
// ... extra code
fjordToken.safeTransfer(msg.sender, total);
points.onUnstaked(msg.sender, _amount);
}

Note that FjordPoints will distribute rewards to users for the epochs after their join. This means that if User1 joins in epoch 4, they would start earning rewards for epoch 5 and onward. This is to avoid just-in-time liquidity and users earning rewards for quickly entering and leaving the pools. This is handled inside updatePendingPoints, where a user’s lastPointsPerToken is synced to the global pointsPerToken.

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;
_;
}

When we look into distributePoints, we can see two if checks verifying whether the first epoch has started and if there is any totalStaked amount. Even if we are in epoch N, but have 0 totalStaked, we won’t update any variables and just return.

function distributePoints() public {
if (block.timestamp < lastDistribution + EPOCH_DURATION) {
return;
}
​
if (totalStaked == 0) {
return;
}

This would enable the first user to stake in epoch 2 and earn all the rewards from epoch 1. While this might be rare, if it happens, this user would earn points from an epoch they were not staked for.

This happens because onStake will first sync the user and then update totalStaked, but since totalStaked == 0 during distributePoints (called in checkDistribution), the user will not be properly synced.

function onStaked(address user, uint256 amount)
external
onlyStaking
checkDistribution
updatePendingPoints(user)
{
UserInfo storage userInfo = users[user];
userInfo.stakedAmount = userInfo.stakedAmount.add(amount);
totalStaked = totalStaked.add(amount);
emit Staked(user, amount);
}

Example:

  1. Epoch 1 passes without anyone staking.

  2. User1 stakes in epoch 2 and gets their lastPointsPerToken synced to 0 since pointsPerToken hasn't been updated yet.

  3. User2 stakes right after them.

  4. User2 will trigger checkDistribution, updating pointsPerToken to its new value and syncing with them.

  5. Since User2 is synced to pointsPerToken of epoch 2, they will not get any rewards.

  6. User1 still has their lastPointsPerToken set to 0, so they can claim all of pointsPerToken, i.e., the rewards for epochs 1 and 2.

Root Cause

  1. The if statement inside distributePoints.

if (totalStaked == 0) {
return;
}

  1. The functions sync before the user increases totalStaked.

function onStaked(address user, uint256 amount)
external
onlyStaking
checkDistribution
updatePendingPoints(user)
{

  1. onStaked does not check for the first depositor depositing late.

Tools Used

Manual review.

Recommendations

If the first depositor triggers distributePoints after their deposit.

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.