Summary
Adding reward to any epoch which started with 0 stakers will result with those reward assets being locked in the contract. The reward is calculated based on the difference between the rewardPerToken from the current epoch and the one where the deposit occurred, so effectively reward assets will not be claimable in a such scenario and will remain locked in the contract.
Vulnerability Details
FjordStaking.sol#L734
FjordStaking.sol#L743
For example, if rewardAdmin adds a reward in the first epoch (since nothing prevents him from doing so), no one will be able to take that reward and the tokens will be locked on the contract. Same issue applies to any other epoch which started without stakers.
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);
}
When the current epoch ends, _checkEpochRollower() will update rewardPerToken array FjordStaking.sol#L707.
What prevents the user from taking the given reward, even though he was the first to stake, is the way in which the reward is calculated in the function FjordStaking:_redeem, which records the reward for each user.
function _redeem(address sender) internal {
UserData storage ud = userData[sender];
ud.unclaimedRewards +=
calculateReward(ud.totalStaked, ud.lastClaimedEpoch, currentEpoch - 1);
ud.lastClaimedEpoch = currentEpoch - 1;
if (ud.unredeemedEpoch > 0 && ud.unredeemedEpoch < currentEpoch) {
DepositReceipt memory deposit = deposits[sender][ud.unredeemedEpoch];
@> ud.unclaimedRewards += calculateReward(
deposit.staked + deposit.vestedStaked, ud.unredeemedEpoch, currentEpoch - 1;
);
ud.unredeemedEpoch = 0;
ud.totalStaked += (deposit.staked + deposit.vestedStaked);
}
}
function calculateReward(uint256 _amount, uint16 _fromEpoch, uint16 _toEpoch)
internal
view
returns (uint256 rewardAmount)
{
@> rewardAmount =
(_amount * (rewardPerToken[_toEpoch] - rewardPerToken[_fromEpoch])) / PRECISION_18;
}
Let's assume a scenario where the user staked in the first epoch and a reward was added in that epoch. After that no reward was added. In the 7th epoch, the user wants to unstake what he staked in the first phase, whereby the FjordStaking::redeem function is triggered. Inside the body of the FjordStaking::redeemfunction, the FjordStaking::calculateRewardfunction is called where the parameter _fromEpoch is the current 7th epoch and _toEpoch is the first epoch. Since in the formula for calculating rewardAmount the value of rewardPerToken[_toEpoch] is the same as rewardPerToken[_fromEpoch], the reward will be zero. In this example, we see that the reward from the first phase will remain locked.
PoC
Place the following test in test/unit/unstake.t.sol:
function testUserCantClaimRewardFromDepositedEpoch() public {
address user=makeAddr("user");
deal(address(token), user, 2 ether);
uint256 userBalance=token.balanceOf(user);
console.log("Balanace of user at the begining:", userBalance);
uint256 contractBalance=token.balanceOf(address(fjordStaking));
console.log("Balanace of contract at the begining:", contractBalance);
uint256 epochBefore = fjordStaking.currentEpoch();
console.log("First stake epoch: ", epochBefore);
vm.startPrank(user);
token.approve(address(fjordStaking), 1 ether);
fjordStaking.stake(1 ether);
vm.stopPrank();
userBalance = token.balanceOf(user);
console.log("Balanace of user after stake:", userBalance);
contractBalance=token.balanceOf(address(fjordStaking));
console.log("Balanace of contract after stake:", contractBalance);
vm.prank(minter);
fjordStaking.addReward(1 ether);
contractBalance = token.balanceOf(address(fjordStaking));
console.log("Balanace of contract after adding reward:",contractBalance);
vm.warp(vm.getBlockTimestamp() + 7 * fjordStaking.epochDuration());
(uint256 totalStakedBefore, uint256 unclaimedRewardsBefore, uint16 unredeemedEpochBefore, uint16 lastClaimedEpochBefore) = fjordStaking.userData(user);
console.log("-----------------------------------------");
console.log("totalStakedBefore", totalStakedBefore);
console.log("unclaimedRewardsBefore", unclaimedRewardsBefore);
console.log("unredeemedEpochBefore", unredeemedEpochBefore);
console.log("lastClaimedEpochBefore", lastClaimedEpochBefore);
console.log("-----------------------------------------");
vm.startPrank(user);
fjordStaking.unstake(1, 1 ether);
vm.stopPrank();
userBalance = token.balanceOf(user);
console.log("Balanace of user after unstaking:", userBalance);
contractBalance = token.balanceOf(address(fjordStaking));
console.log("Balanace of contract after unstaking:", contractBalance);
console.log("-----------------------------------------");
(uint256 totalStaked, uint256 unclaimedRewards, uint16 unredeemedEpoch, uint16 lastClaimedEpoch) = fjordStaking.userData(user);
console.log("totalStaked", totalStaked);
console.log("unclaimedRewards", unclaimedRewards);
console.log("unredeemedEpoch", unredeemedEpoch);
console.log("lastClaimedEpoch", lastClaimedEpoch);
console.log("-----------------------------------------");
vm.startPrank(user);
vm.expectRevert();
fjordStaking.claimReward(false);
vm.stopPrank();
}
Impact
If reward is added before or in the same epoch as when the first stake occurs, those reward funds will be locked in the contract.
Tools Used
Manual code review / Foundry tests
Recommendations
In the FjordStaking::addReward function, add a condition that will make it impossible to add a reward, if there is was no stakers at the start of the epoch.
