Summary
The FjordStaking contract contains a vulnerability in the completeClaimRequest function where users are required to wait for one additional epoch beyond the intended claim cycle before they can claim their rewards. This discrepancy between the intended and actual behavior could lead to user frustration and potential loss of trust in the platform.
Vulnerability Details
Relavant link - https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L662-L687
In the completeClaimRequest function, the check for the claim cycle is implemented as highlighted below:
function completeClaimRequest()
external
checkEpochRollover
redeemPendingRewards
returns (uint256 rewardAmount)
{
ClaimReceipt memory cr = claimReceipts[msg.sender];
if (cr.requestEpoch < 1) revert ClaimReceiptNotFound();
@> if (currentEpoch - cr.requestEpoch <= claimCycle) revert CompleteRequestTooEarly();
rewardAmount = cr.amount;
userData[msg.sender].unclaimedRewards -= rewardAmount;
totalRewards -= rewardAmount;
delete claimReceipts[msg.sender];
fjordToken.safeTransfer(msg.sender, rewardAmount);
emit RewardClaimed(msg.sender, rewardAmount);
}
This implementation meant to be users to wait for 3 epochs after their claim request before they can complete the claim. However due to current implementation users have to wait for 4 epoches, instead of the intended 3 epochs.
The condition <= should be < to allow claims after 3 epochs have passed.
POC
In base test, comment out
We have make to some modification in test suite file as well. In claimReward.t.sol
add an import
+ import {console2} from "forge-std/console2.sol";
Then add the following test:
function test_ClaimReward_Timing() public {
vm.startPrank(alice);
token.approve(address(fjordStaking), 100 ether);
fjordStaking.stake(100 ether);
vm.stopPrank();
_addRewardAndEpochRollover(1 ether, 3);
vm.prank(alice);
fjordStaking.claimReward(false);
uint16 requestEpoch = fjordStaking.currentEpoch();
_addRewardAndEpochRollover(1 ether, 2);
vm.expectRevert(FjordStaking.CompleteRequestTooEarly.selector);
vm.prank(alice);
fjordStaking.completeClaimRequest();
_addRewardAndEpochRollover(1 ether, 1);
console2.log("Current Epoch:", fjordStaking.currentEpoch(), "Request Epoch:", requestEpoch);
vm.prank(alice);
vm.expectRevert(FjordStaking.CompleteRequestTooEarly.selector);
fjordStaking.completeClaimRequest();
console2.log("Claim failed after 3 epochs as it should not in ideal scenerio");
_addRewardAndEpochRollover(1 ether, 1);
vm.prank(alice);
uint256 claimedAmount = fjordStaking.completeClaimRequest();
console2.log("Claim succeeded after 4 epochs. Claimed amount:", claimedAmount);
}
run the forge test --mt test_ClaimReward_Timing -vv in the terminal, and it will show following results:
[⠒] Compiling...
[⠒] Compiling 1 files with Solc 0.8.21
[⠑] Solc 0.8.21 finished in 2.48s
Compiler run successful!
Ran 1 test for test/unit/claimReward.t.sol:ClaimReward_Unit_Test
[PASS] test_ClaimReward_Timing() (gas: 583101)
Logs:
Current Epoch: 7 Request Epoch: 4
Claim failed after 3 epochs as it should not in ideal scenerio
Claim succeeded after 4 epochs. Claimed amount: 2727272727272727100
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.03ms (746.46µs CPU time)
Impact
The impact of this vulnerability is low. While it does not result in loss of funds or compromise the security of the contract, it does cause an unintended delay in users ability to claim their rewards. This could lead to:
Inconsistency between documented behavior and actual contract functionality.
Temporary loss of liquidity for users expecting to access their rewards after the intended 3-epoch wait.
Tools Used
Manual Review, Foundry
Recommendations
To fix this issue, modify the condition in the completeClaimRequest function as shown below:
function completeClaimRequest()
external
checkEpochRollover
redeemPendingRewards
returns (uint256 rewardAmount)
{
ClaimReceipt memory cr = claimReceipts[msg.sender];
//CHECK
if (cr.requestEpoch < 1) revert ClaimReceiptNotFound();
// to complete claim receipt, user must wait for at least 3 epochs
- if (currentEpoch - cr.requestEpoch <= claimCycle) revert CompleteRequestTooEarly();
+ if (currentEpoch - cr.requestEpoch < claimCycle) revert CompleteRequestTooEarly();
//EFFECT
rewardAmount = cr.amount;
userData[msg.sender].unclaimedRewards -= rewardAmount;
totalRewards -= rewardAmount;
delete claimReceipts[msg.sender];
//INTERACT
fjordToken.safeTransfer(msg.sender, rewardAmount);
emit RewardClaimed(msg.sender, rewardAmount);
}
