Summary
If a user claims rewards early, the protocol imposes a 50% penalty. However, due to rounding down during penalty calculation, the actual penalty amount may be slightly reduced, benefiting the user.
Vulnerability Details
The Fjord Protocol allows users to claim their rewards early, but it imposes a 50% penalty for doing so. If a staker wishes to claim their rewards early, they can set isClaimEarly=true when calling the claimReward function:
function claimReward(bool _isClaimEarly)
external
checkEpochRollover
redeemPendingRewards
returns (uint256 rewardAmount, uint256 penaltyAmount)
{
...
if (!_isClaimEarly) {
claimReceipts[msg.sender] =
ClaimReceipt({ requestEpoch: currentEpoch, amount: ud.unclaimedRewards });
emit ClaimReceiptCreated(msg.sender, currentEpoch);
return (0, 0);
}
rewardAmount = ud.unclaimedRewards;
@>-- penaltyAmount = rewardAmount / 2;
rewardAmount -= penaltyAmount;
if (rewardAmount == 0) return (0, 0);
totalRewards -= (rewardAmount + penaltyAmount);
userData[msg.sender].unclaimedRewards -= (rewardAmount + penaltyAmount);
fjordToken.safeTransfer(msg.sender, rewardAmount);
emit EarlyRewardClaimed(msg.sender, rewardAmount, penaltyAmount);
}
The code demonstrates that the penalty amount calculation rounds down.
For example, if a user wants to claim their rewards = 19209357049074421247685465835409 with idClaimEarly = true, the penaltyAmount would be 9604678524537210623842732917704.5. However, since Solidity rounds down by default, the penaltyAmount becomes 9604678524537210623842732917704. This results in a loss of 1 wei for the protocol.
While this loss is minimal for a single claim, it could accumulate over time with multiple occurrences, potentially leading to a significant total loss.
POC
A staeLess fuzz test which will help to proof my report :
function test_penaltyAmount(uint256 a) public {
a = bound(a, 1e6, 2e18);
assertEq(a/2 , (a+1)/2);
}
run with command : forge test --mt test_penaltyAmount -vvv.
Traces:
[7504] TestReq::test_penaltyAmount(19209357049074421247685465835409 [1.92e31])
├─ [0] console::log("Bound result", 25916605329310867 [2.591e16]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::assertEq(12958302664655433 [1.295e16], 12958302664655434 [1.295e16]) [staticcall]
│ └─ ← [Revert] assertion failed: 12958302664655433 != 12958302664655434
└─ ← [Revert] assertion failed: 12958302664655433 != 12958302664655434
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 2.09ms (1.31ms CPU time)
Ran 1 test suite in 7.94ms (2.09ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/tokenApproval.t.sol:TestReq
[FAIL. Reason: assertion failed: 12958302664655433 != 12958302664655434; counterexample: calldata=0x8318f85300000000000000000000000000000000000000f274c7bb08cfb1c6d56b2d8791 args=[19209357049074421247685465835409 [1.92e31]]] test_penaltyAmount(uint256) (runs: 3, μ: 7505, ~: 7506)
Impact
Due to rounding down, the penaltyAmount ends up benefiting the user instead of Fjord.
Tools Used
Manual Review
Recommendations
instead of round down round up in favour of Fjord as follows :
@@ -642,7 +643,7 @@ contract FjordStaking is ISablierV2LockupRecipient {
}
rewardAmount = ud.unclaimedRewards;
- penaltyAmount = rewardAmount / 2;
+ penaltyAmount = (rewardAmount+1) / 2;
rewardAmount -= penaltyAmount;
