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

The `penaltyAmount` would be deducted from the `rewardAmount` - even when a user does not claim early

0xmuxyz

Summary

Within the FjordStaking#claimReward(), the penaltyAmount is supposed to be deducted from the rewardAmount only when a user claim early.

However, within the FjordStaking#claimReward(), the penaltyAmount would be deducted from the rewardAmount even when a user does not claim early (which the user claim after the reward cooldown period elapsed).

Vulnerability Details

When a user would receive the rewards from their staking, the user would call the FjordStaking#claimReward().

Within the FjordStaking#claimReward(), the half of the rewardAmount (rewardAmount / 2) would be stored into the penaltyAmount.
Then, the penaltyAmount would be subtracted from the rewardAmount (rewardAmount -= penaltyAmount).
Finally, the rewardAmount left (subtracted) would be transferred to the user (msg.sender) like this:
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L645-L646
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L654

/// @notice Claim reward from specific epoch.
/// @dev This function allows users to claim rewards from an epochs,
/// if the user chooses to bypass the reward cooldown of 3 epochs, ///<------ @audit - "3 epochs" means "21 days (== 3 weeks)"
/// then reward penalty will be levied. ///<---------------------------- @audit
/// @param _isClaimEarly Whether user wants to claim early and incur penalty.
/// @return rewardAmount The reward amount that has been distributed.
/// @return penaltyAmount The penalty incurred by the user for early claim.
function claimReward(bool _isClaimEarly)
external
checkEpochRollover
redeemPendingRewards
returns (uint256 rewardAmount, uint256 penaltyAmount)
{
//CHECK
UserData storage ud = userData[msg.sender];
​
// do not allow to claimReward while user have pending claimReceipt
// or user have claimed from the last epoch
if (
claimReceipts[msg.sender].requestEpoch > 0
|| claimReceipts[msg.sender].requestEpoch >= currentEpoch - 1
) revert ClaimTooEarly();
​
if (ud.unclaimedRewards == 0) revert NothingToClaim();
​
//EFFECT
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; ///<------ @audit - 50% of rewardAmount will be distributed as a "Penalty Fee" to the "xFJO Stakers".
rewardAmount -= penaltyAmount; ///<-------- @audit - The "Penalty Fee" above would be subtracted from the "rewardAmount"
​
if (rewardAmount == 0) return (0, 0);
​
totalRewards -= (rewardAmount + penaltyAmount);
userData[msg.sender].unclaimedRewards -= (rewardAmount + penaltyAmount);
​
//INTERACT
fjordToken.safeTransfer(msg.sender, rewardAmount); ///<-------- @audit
...

According to the NatSpec of the FjordStaking#claimReward() above, the penalty is supposed to be applied to the user's rewards - if the user would claim earlier than the cooldown period of 3 epochs (21 days) like this:

/// if the user chooses to bypass the reward cooldown of 3 epochs,
/// then reward penalty will be levied.

Also, according to the diagram of the "Rewards Issuance" in the code walkthrough (and discord), only when a user would claim early (< 21 days), 50% of rewardAmount is supposed to be sent to the xFJO stakers as a penalty.

However, within the FjordStaking#claimReward(), the penaltyAmount would always be subtracted from the rewardAmount - even if a user would not claim early (which the user claim after the reward cooldown period elapsed)

Impact

This lead to unintended-situation that even if a user would not claim early, the penaltyAmount would be subtracted from the rewardAmount of the user.
As a result, the user would receive only 50% of the rewardAmount (in other words, the user would lose the 50% of the rewardAmount that the user was supposed to receive) - even though the user would claim after the reward cooldown period elapsed (which does not claim early).

The severity of this vulnerability should be considered as a "High". Because sophisticated users, who claim their rewards after the cooldown period elapsed, would always lose 50% of their rewardAmount.

Tools Used

  • Foundry

Recommendations

Within the FjordStaking#claimReward(), consider adding the else block to the if (_isClaimEarly == True) block.
Then, consider moving the penaltyAmount calculation and the penaltyAmount deduction (from the rewardAmount) to the else block - so that the penaltyAmount would be applied (deducted from the rewardAmount) only when a user claim early like this:

function claimReward(bool _isClaimEarly)
external
checkEpochRollover
redeemPendingRewards
returns (uint256 rewardAmount, uint256 penaltyAmount)
{
//CHECK
UserData storage ud = userData[msg.sender];
...
​
+ rewardAmount = ud.unclaimedRewards;
​
//EFFECT
if (!_isClaimEarly) {
claimReceipts[msg.sender] =
ClaimReceipt({ requestEpoch: currentEpoch, amount: ud.unclaimedRewards });
​
+ //INTERACT
+ fjordToken.safeTransfer(msg.sender, rewardAmount)
​
emit ClaimReceiptCreated(msg.sender, currentEpoch);
​
return (0, 0);
- }
+ } else { /// @dev - if (_isClaimEarly == True)
+ rewardAmount = ud.unclaimedRewards;
+ penaltyAmount = rewardAmount / 2;
+ rewardAmount -= penaltyAmount;
+
+ //INTERACT
+ fjordToken.safeTransfer(msg.sender, rewardAmount)
+ }
​
- 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);
...
Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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