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

Users can frontrun `Points` distribution without needing to lock tokens for the `lockCycle` in `FjordStaking` under specific edge case scenarios.

Summary

If FjordPoints and FjordStaking are deployed in different blocks, resulting in different block timestamps, the epoch switch will occur at different times. This discrepancy creates an opportunity for Points farming without requiring token locking for the lockCycle, allowing users to frontrun the Points distribution.

Vulnerability Details

Because FjordPoints and FjordStaking contracts are deployed independently, they might be deployed in different blocks with varying block timestamps.

Examining the deployment script:

File: DeployStaking.s.sol
10: function run() public {
11: vm.startBroadcast();
12:
13: FjordPoints points = new FjordPoints();
14:
15: AuctionFactory factory = new AuctionFactory(address(points));
16:
17: FjordStaking staking = new FjordStaking(
18: vm.envAddress("FJO_ADDRESS"),
19: msg.sender,
20: vm.envAddress("SABLIERV2_LOCKUPLINEAR"),
21: vm.envAddress("AUTHORIZED_SENDER"),
22: address(points)
23: );

Here, FjordPoints is deployed before FjordStaking. Although Foundry broadcasts these transactions sequentially, there is no guarantee that the FjordStaking deployment will occur in the same block as the FjordPoints deployment. In an edge case, FjordPoints could be included in one block, and due to a gas spike, FjordStaking might be deployed several blocks later.

If FjordPoints and FjordStaking are deployed in different blocks, their epoch start times will differ. This discrepancy can be exploited to farm Points without locking tokens for the lockCycle in FjordStaking.

This is possible because in the FjordStaking contract, users can stake and unstake FJO tokens immediately within the same epoch, as shown in this code:

File: FjordStaking.sol
463: if (currentEpoch != _epoch) { // <== can stake and unstake within the same epoch.
464: // If _epoch is less than the current epoch, the user can unstake after the complete lockCycle.
465: if (currentEpoch - _epoch <= lockCycle) revert UnstakeEarly();
466: }

Consider this scenario:

  1. Alice notices that FjordPoints and FjordStaking are deployed in different blocks, causing their epoch switches to differ.

  2. Alice stakes a large amount of FJO in FjordStaking just before the epoch switch in the FjordPoints contract.

  3. Alice calls FjordPoints.distributePoints() in the first block that allows for new Points distribution (after the epoch switch in the FjordPoints contract) and then calls the unstake function in the same block before the epoch switch in the FjordStaking contract.

  4. Alice is awarded Points from FJO that she didn't need to stake for the full lockCycle duration.

There is no frontrunning protection mechanism for Points distribution other than locking tokens for the lockCycle in the FjordStaking contract, which can be circumvented as shown.

In the FjordStaking contract, frontrunning protection is present in two forms: new rewards are only distributed to stakers from the previous epoch and only for tokens locked for the full lockCycle duration.

Impact

  • Frontrunning of the Points distribution in the FjordPoints contract.

Code Snippet

https://github.com/Cyfrin/2024-08-fjord/blob/main/script/forge/DeployStaking.s.sol#L10-L25
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L462-L466
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordPoints.sol#L232-L248

Tools Used

Manual review.

Recommendations

The epoch switch times should be aligned for both FjordPoints and FjordStaking contracts.

This can be achieved by either deploying the FjordPoints contract within the FjordStaking contract's constructor:

File: FjordStaking.sol
-299: points = IFjordPoints(_fjordPoints);
+299: points = IFjordPoints(new FjordPoints(owner)); // <== FjordPoints should be modified to accept the owner address
300: if (_authorizedSablierSender != address(0)) {
301: authorizedSablierSenders[_authorizedSablierSender] = true;
302: }
303: }

Or by updating the lastDistribution with FjordStaking.startTime() when calling the FjordPoints.setStakingContract() function:

File: FjordPoints.sol
172: function setStakingContract(address _staking) external onlyOwner {
173: if (_staking == address(0)) {
174: revert InvalidAddress();
175: }
176:
177: staking = _staking;
+ uint256 startTime = staking.startTime();
+ uint256 weeksPassed = (block.timestamp - startTime) / EPOCH_DURATION;
+ lastDistribution = startTime + (weeksPassed * EPOCH_DURATION);
178: }
Updates

Lead Judging Commences

inallhonesty Lead Judge
9 months ago
inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Appeal created

eeyore Submitter
9 months ago
inallhonesty Lead Judge
9 months ago
inallhonesty Lead Judge 9 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.