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

Based on The developers flow a user should claim his rewards immediately he unstakeAll but we Fail to Claim Rewards Upon Unstaking Vested in Staking Contract

bigsam

Summary

The current implementation of the staking contract allows users to unstakeAll tokens without automatically claiming their pending rewards. This issue can lead to a scenario where users, who would normally incur a penalty for early unstakingall and reward claiming, can bypass the penalty. As a result, users may unintentionally or deliberately avoid paying the penalty

Vulnerability Details

The unstaking process in the contract does not automatically trigger a reward claim, even though the unstakeall action should ideally be coupled with an immediate reward claim.

​
​
/// @notice Unstake from all epochs.
​
@audit issue>>>> /// @dev This function allows users to unstake from all the epochs at once,
@audit issue>>>> /// while also claiming all the pending rewards. ///NOTE
​
/// @return totalStakedAmount The total amount that has been unstaked.
function unstakeAll()
external
checkEpochRollover
redeemPendingRewards
returns (uint256 totalStakedAmount)
{
uint256[] memory activeDeposits = getActiveDeposits(msg.sender);
if (activeDeposits.length == 0) revert NoActiveDeposit();
​
for (uint16 i = 0; i < activeDeposits.length; i++) {
uint16 epoch = uint16(activeDeposits[i]);
DepositReceipt storage dr = deposits[msg.sender][epoch];
​
if (dr.epoch == 0 || currentEpoch - epoch <= lockCycle) continue;
​
totalStakedAmount += dr.staked;
​
// no vested staked and stake is 0 then delete the deposit
if (dr.vestedStaked == 0) {
delete deposits[msg.sender][epoch];
_activeDeposits[msg.sender].remove(epoch);
} else {
// still have vested staked, then only delete the staked
dr.staked = 0;
}
}
​
totalStaked -= totalStakedAmount;
userData[msg.sender].totalStaked -= totalStakedAmount;
​
fjordToken.transfer(msg.sender, totalStakedAmount);
points.onUnstaked(msg.sender, totalStakedAmount);
​
// emit event
emit UnstakedAll(
msg.sender, totalStakedAmount, activeDeposits, getActiveDeposits(msg.sender)
);
}
​
​

The function is designed to transfer the unstaked tokens back to the user and record the unstake event. However, there is no direct call to claim the user's rewards immediately after unstaking. This omission allows users to unstake their tokens without claiming their pending rewards, effectively evading the early claim penalty.

Additionally, the `claimReward` function checks whether the user wants to claim early and applies a penalty accordingly:

​
function claimReward(bool \_isClaimEarly)
​
external
​
checkEpochRollover
​
redeemPendingRewards
​
returns (uint256 rewardAmount, uint256 penaltyAmount)
​
{
​
// (Logic for claiming rewards)
​
}
​

Since the `claimReward` function is separate from the `unstakeAll` function, a user who unstakes his vested NFT but does not call `claimReward` immedaitely can avoid paying the early claim penalty.

Impact

This vulnerability enables users to bypass the penalty for early reward claiming by unstaking their tokens without immediately claiming their rewards.

Tools Used

Manual Code Review

Recommendations

1. **Enforce Immediate Reward Claim on Unstaking**: Modify the `unstakeAll` function to include an automatic call to the `claimReward` function, ensuring that users cannot unstakeAll tokens without claiming their rewards. This will enforce the early claim penalty as intended.

Updates

Lead Judging Commences

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.