Summary
In a different issue I identified a vulnerability in the addReward function, where front-running the function could prevent the condition if (latestEpoch > currentEpoch) in _checkEpochRollover() from executing, leading to delayed reward distributions. Once this issue is resolved and the epoch rollover is correctly triggered, a new issue from a different root cause becomes evident.
In the updated code, when the condition if (latestEpoch > currentEpoch) passes, the totalRewards gets updated correctly. However, the loop intended to update rewardPerToken does not execute if lastEpochRewarded + 1 equals currentEpoch. This results in a scenario where totalRewards is updated, but rewardPerToken is not, leading to permanently lost rewards.
Vulnerability Details
Upon fixing the previously identified issue where front-running the addReward function prevented epoch rollover, a new vulnerability emerges. Specifically, when the if (latestEpoch > currentEpoch) condition in the _checkEpochRollover() function passes, totalRewards is correctly updated, but rewardPerToken may not be updated due to the loop condition.
Updating totalRewards:
https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L702C17-L705C48
uint256 pendingRewards = (currentBalance + totalVestedStaked + newVestedStaked)
- totalStaked - newStaked - totalRewards;
uint256 pendingRewardsPerToken = (pendingRewards * PRECISION_18) / totalStaked;
totalRewards += pendingRewards;
In this segment, totalRewards is increased by pendingRewards.
Failure to Update rewardPerToken
https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L706C17-L709C18
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
This loop will not execute if lastEpochRewarded + 1 equals currentEpoch, meaning rewardPerToken does not get updated for the new epoch.
POC
Updated addReward that triggers the inner logic of _checkEpochRollover
function addReward(uint256 _amount) external onlyRewardAdmin {
if (_amount == 0) revert InvalidAmount();
currentEpoch--;
fjordToken.safeTransferFrom(msg.sender, address(this), _amount);
_checkEpochRollover();
emit RewardAdded(currentEpoch, msg.sender, _amount);
}
Foundry test
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: 342181)
Logs:
fjordStaking.rewardPerToken(1) 0
fjordStaking.rewardPerToken(2) 200000000000000000
fjordStaking.rewardPerToken(3) 300000000000000000
fjordStaking.rewardPerToken(4) 400000000000000000
[PASS] test_frontrun() (gas: 1077331)
Logs:
fjordStaking.rewardPerToken(1) 0
fjordStaking.rewardPerToken(2) 0
fjordStaking.rewardPerToken(3) 0
fjordStaking.rewardPerToken(4) 0
Impact
Lost Rewards: The expected rewards per token (rewardPerToken) are not updated, causing the rewards to be lost permanently.
Tools Used
Foundry
Recommendations
Modify the _checkEpochRollover function to ensure that the rewardPerToken array is updated even if lastEpochRewarded + 1 equals currentEpoch.
function _checkEpochRollover() internal {
uint16 latestEpoch = getEpoch(block.timestamp);
if (latestEpoch > currentEpoch) {
//Time to rollover
currentEpoch = latestEpoch;
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]);
}
++ rewardPerToken[currentEpoch - 1] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
} else {
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded];
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
}
totalStaked += newStaked;
totalVestedStaked += newVestedStaked;
newStaked = 0;
newVestedStaked = 0;
lastEpochRewarded = currentEpoch - 1;
}
}
same test logs after update
[PASS] test_Correct_rewards() (gas: 346819)
Logs:
fjordStaking.rewardPerToken(1) 0
fjordStaking.rewardPerToken(2) 200000000000000000
fjordStaking.rewardPerToken(3) 300000000000000000
fjordStaking.rewardPerToken(4) 400000000000000000
[PASS] test_frontrun() (gas: 1226680)
Logs:
fjordStaking.rewardPerToken(1) 100000000000000000
fjordStaking.rewardPerToken(2) 199999999999999999
fjordStaking.rewardPerToken(3) 299999999999999998
fjordStaking.rewardPerToken(4) 399999999999999997
