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

Allowing users to stake and unstake in thesame epoch can open a vulnerability in the FjordPoints.sol contract making users earn point without staking for long.

joshuajee

Summary

Stakers earn both Fjord Token and points when they stake on the FjordStaking staking contract. A user can stake and unstake their Fjord token in the same epoch, this exposes a vulnerability in the FjordPoints.sol contract, as the user can stake a few minutes before points are distributed and earn points as if they have staked for 1 whole week. The moment points are distributed they will claim their points and unstake immediately, this can be done within 5 minutes. A user who stakes for just 5 minutes will get the same rewards as someone who has been in the system for days and also remove their staked tokens, which is like receiving free money.

Vulnerability Details

The stake function calls the points.onStaked(msg.sender, _amount)function on the point contracts, which records the amount of the user's stake in the point contract, this amount is used to distribute rewards.

So users earn per staked amount.

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L388

function stake(uint256 _amount) external checkEpochRollover redeemPendingRewards {
...
fjordToken.safeTransferFrom(msg.sender, address(this), _amount);
@-> points.onStaked(msg.sender, _amount);
​
emit Staked(msg.sender, currentEpoch, _amount);
}

Let's take a look at the onStaked function, it updates the user staked amount and total Staked in the contract, it also runs the checkDistribution and updatePendingPoints modifier.

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

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

The checkDistribution calls the distributePoints function, let's take a look at the distribution function we can see that the distributePoints function only updates the rewards when the last distribution time plus the EPOCH_DURATION (1 week) is greater than the current block.timestamp.

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

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

So if the user stakes 3 minutes to the end of the epoch the distributePoints won't update the pointsPerToken i.e the block.timestamp is 3 minutes less than lastDistribition + EPOCH_DURATION, the user can wait for 3 minutes after staking to call the distributePointsso they can earn points in just 5 minutes of staking.

After they have claimed their rewards on the point contract they can call the unstake function on the staking contract and withdraw their tokens. This transaction will go through because the staking and unstaking are done on the same epoch.

Check the unstake function here, we can see that it allows users to unstake in the same epoch that they staked on.

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L473

function unstake(uint16 _epoch, uint256 _amount)
external
checkEpochRollover
redeemPendingRewards
returns (uint256 total)
{
...
// _epoch is same as current epoch then user can unstake immediately
if (currentEpoch != _epoch) {
// _epoch less than current epoch then user can unstake after at complete lockCycle
@-> if (currentEpoch - _epoch <= lockCycle) revert UnstakeEarly();
}
​
//EFFECT
dr.staked -= _amount;
//@audit why?
if (currentEpoch != _epoch) {
totalStaked -= _amount;
userData[msg.sender].totalStaked -= _amount;
} else {
// unstake immediately
@-> newStaked -= _amount;
}
​
...
}
​

Impact

  1. Staker will earn rewards even for periods they didn't stake for.

  2. People can earn points without locking their funds.

Tools Used

Manual Analysis.

Recommendations

Do not allow users to stake and unstake in the same epoch that they staked for. Do this for the unstake and unstakeVested function.

if (currentEpoch != _epoch) {
totalStaked -= _amount;
userData[msg.sender].totalStaked -= _amount;
} else {
- // unstake immediately
- newStaked -= _amount;
+ revert UnstakeEarly();
}

Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

joshuajee Submitter
10 months ago
inallhonesty Lead Judge
10 months ago
joshuajee Submitter
10 months ago
joshuajee Submitter
10 months ago
joshuajee Submitter
10 months ago
joshuajee Submitter
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

If epoch end times of FjordStaking and FjordPoints are desynchronized, users will be able to exploit the desynchronization to stake>claim>unstake instantly, getting points they shouldn't

Impact: High - Users are getting an unreasonable amount of points through exploiting a vulnerability Likelihood: Low - Most of the times, when using the script, all deployment tx will get processed in the same block. But, there is a small chance for them to be processed in different blocks.

Support

FAQs

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