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

FjordStaking.sol: unstakeAll does not unstake current epoch

trooper

Summary

In unstakeAll the contract unstakes each active epoch. But it skips the current epoch, even though it would be possible to unstake that.

Vulnerability Details

The comparison if (dr.epoch == 0 || currentEpoch - epoch <= lockCycle) continue; source is true for currentEpoch == epoch. Therefor the current epoch will be skipped and not unstaked.

Impact

Users might not notice that the unstakeAll does not work as indented and could think they unstaked everything. If they dont call unstake with the current epoch, the tokens would stay in the contract.

Tools Used

manual review

Recommendations

Change unstakeAll to also unstake the current epoch. For example:

--- a/FjordStaking.sol.orig
+++ b/FjordStaking.sol
@@ -597,6 +597,26 @@ contract FjordStaking is ISablierV2LockupRecipient {
totalStaked -= totalStakedAmount;
userData[msg.sender].totalStaked -= totalStakedAmount;
​
+ // unstake current epoch
+ if (_activeDeposits[msg.sender].contains(currentEpoch)) {
+ DepositReceipt storage dr = deposits[msg.sender][currentEpoch];
+
+ uint256 amount = dr.staked;
+ totalStakedAmount += amount;
+
+ // unstake immediately
+ newStaked -= amount;
+
+ // no vested staked and stake is 0 then delete the deposit
+ if (dr.vestedStaked == 0) {
+ delete deposits[msg.sender][currentEpoch];
+ _activeDeposits[msg.sender].remove(currentEpoch);
+ } else {
+ // still have vested staked, then only delete the staked
+ dr.staked = 0;
+ }
+ }
+
fjordToken.transfer(msg.sender, totalStakedAmount);
points.onUnstaked(msg.sender, totalStakedAmount);
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.