DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: medium
Invalid

`penaltyAmount` levied on early claimers is also considered as `reward`

Summary

penaltyAmount levied on early claimers is also considered as reward

Vulnerability Details

Users can claim their reward early using fjordStaking:claimReward() with _isClaimEarly = true as input. While claiming early, they pay 50% of their unclaimedRewards as penaltyAmount.

function claimReward(bool _isClaimEarly)
external
checkEpochRollover
redeemPendingRewards
returns (uint256 rewardAmount, uint256 penaltyAmount)
{
...
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);
}

If we see above code then we can see, only rewardAmount(ie 50% of unclaimedReward) is transfered to msg.sender, which means penaltyAmount is still in the stakingContract but totalRewards is reduced by rewardAmount + penaltyAmount

Now this is a problem because totalRewards is used for calculating pendingRewards in _checkEpochRollover(). Any extra amount of fjordToken in stakingContract is considered as reward, which is calculated by subtracting totalRewards from balanceOf stakingContract, and we've seen above penaltyAmount is still in the contract but totalRewards is reduced by penaltyAmount, which means penaltyAmount is extra token and will be considered as reward

function _checkEpochRollover() internal {
...
@> uint256 currentBalance = fjordToken.balanceOf(address(this));
// no distribute the rewards to the users coming in the current epoch
@> uint256 pendingRewards = (currentBalance + totalVestedStaked + newVestedStaked)
- totalStaked - newStaked - totalRewards;
uint256 pendingRewardsPerToken = (pendingRewards * PRECISION_18) / totalStaked;
totalRewards += pendingRewards;
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
...
}

Impact

penaltyAmount will also be distributed as reward to stakers

Tools Used

Manual Review

Recommendations

Send penaltyAmount to owner or treasury of the protocol in claimReward()

+ fjordToken.safeTransfer(owner/treasury, penaltyAmount);
Updates

Lead Judging Commences

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.