Fjord Token Staking
Fjord
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Denial of Service (DoS) on `FjordStaking::claimReward` occurs when users set the parameter to false, locking their rewards and preventing them from claiming.

EyanIngles

Summary

Description: When a user utilises FjordStaking::stakeVested to receive rewards, the natspec suggests that the user should be able to partially claim rewards (with a penalty) or claim the full amount after the penalty period ends. However, if the user sets the parameter to false, it returns 0 reward points and locks the rewards away. If the user attempts to claim with a penalty (by setting the parameter to true), the function reverts, leaving the user unable to retrieve their rewards.

Vulnerability Details

  1. The user stakes assets using FjordStaking::stakeVested and begins earning rewards.

  2. After 7 weeks, the user expects to be able to claim the full reward, having surpassed the early claim period.

  3. The user calls the claimReward function, which executes successfully, but the return value is 0, indicating no rewards were claimed.

  4. The user waits another week, deciding to claim the rewards with a penalty, but when attempting to do so, the function reverts, resulting in no rewards being claimed.

Plug the following testing code in stakeVested.t.sol.

function test_ClaimRewardsLocked() public {
createStreamAndStake();
​
_addRewardAndEpochRollover(1 ether, 5);
assertEq(fjordStaking.currentEpoch(), 6);
​
uint256 streamID = createStreamAndStake();
​
NFTData memory nftData = fjordStaking.getStreamData(address(alice), streamID);
// streamId = 1403;
assertEq(nftData.epoch, 6);
assertEq(nftData.amount, 10 ether);
vm.startPrank(alice);
​
fjordStaking.getStreamOwner(1403);
skip(7 weeks);
fjordStaking.claimReward(false);
skip(1 weeks);
vm.expectRevert();
fjordStaking.claimReward(true);
}

Impact

This flaw in the code can cause users to lock away their assets without receiving any rewards, undermining the primary incentive for staking. It leads to a loss of rewards and erodes trust in the protocol.

Tools Used

Manual Review

Recommendations

  1. I recommend attaching the isClaimEarly flag to the userData struct, associating it directly with each user. After the user has surpassed the 6-week period or double the claimCycle, the boolean should be set to true, allowing the user to access their full rewards.

  2. Additionally, I suggest adding an if statement to handle the isClaimEarly condition. If isClaimEarly is false, the penalty amount should be applied; otherwise, the penalty should be set to 0.

-function claimReward(bool _isClaimEarly)
+function claimReward()
external
checkEpochRollover
redeemPendingRewards
returns (uint256 rewardAmount, uint256 penaltyAmount)
{
//CHECK
UserData storage ud = userData[msg.sender];
if (
claimReceipts[msg.sender].requestEpoch > 0
|| claimReceipts[msg.sender].requestEpoch >= currentEpoch - 1
) revert ClaimTooEarly();
​
if (ud.unclaimedRewards == 0) revert NothingToClaim();
+ if (ud.isClaimEarly == false && ud.unclaimedRewards > (claimCycle * 2)) // doing double the cycle length because it specifies this is half which also occur in penalties.
+ {
+ ud.isClaimEarly = true;
+ }
//EFFECT
- if (!isClaimEarly) {
+ if (!ud.isClaimEarly) {
claimReceipts[msg.sender]
ClaimReceipt({ requestEpoch: currentEpoch, amount: ud.unclaimedRewards });
​
emit ClaimReceiptCreated(msg.sender, currentEpoch);
​
return (0, 0);
}
+ if(ud.isClaimEarly == false ){
+ rewardAmount = ud.unclaimedRewards;
+ penaltyAmount = rewardAmount / 2;
+ rewardAmount -= penaltyAmount;
+ } else {
+ rewardAmount = ud.unclaimedRewards;
+ penaltyAmount = 0;
+ }
- rewardAmount = ud.unclaimedRewards;
- penaltyAmount = rewardAmount / 2;
- rewardAmount -= penaltyAmount;
​
if (rewardAmount == 0) return (0, 0);
​
totalRewards -= (rewardAmount + penaltyAmount);
userData[msg.sender].unclaimedRewards -= (rewardAmount + penaltyAmount);
​
//INTERACT
fjordToken.safeTransfer(msg.sender, rewardAmount);
​
emit EarlyRewardClaimed(msg.sender, rewardAmount, penaltyAmount);
}
struct UserData {
uint256 totalStaked;
uint256 unclaimedRewards;
uint16 unredeemedEpoch;
uint16 lastClaimedEpoch;
+ bool isClaimEarly;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Appeal created

EyanIngles Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.