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

unstakeAll don't unstake a deposit made in the current epoch

0xphantom

Summary

A user can unstake a deposit in 2 situations :

A deposit can be unstaked in two cases: if it has waited for 6 epochs or if the deposit was made during the same epoch as the unstake call.

The problem is that the unstakeAll function(L-570) ignore all the deposits that didn't wait at least 6 epoch.

Vulnerability Details

When a user call unstakeAll it will loop over all the activeDeposit of the sender it will check if the activeDeposit is old enought to unstake it as we can see here :

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;

the proble here is that the if statement will make the function ignore the activeDeposit if it has been made in the same epoch as the unstakeAll call.

This contradicts the unstake function(L-449) as we can see here :

// _epoch is same as current epoch then user can unstake immediately
if (currentEpoch != _epoch) {
// _epoch less than current epoch then user can unstake after at complete lockCycle
if (currentEpoch - _epoch <= lockCycle) revert UnstakeEarly();
}

You can run this test in the StakeRewardScenarios contract in the StakeReward.t.sol file :

function test_StakeAll_POC() public {
fjordStaking.stake(1);
fjordStaking.unstake(1, 1);
(, uint256 stakedAmount,) = fjordStaking.deposits(address(this), 1);
​
assertEq(stakedAmount, 0);
fjordStaking.stake(1);
fjordStaking.unstakeAll();
(, uint256 stakedAmountAfter,) = fjordStaking.deposits(address(this), 1);
​
assertEq(stakedAmountAfter, 1);
}

Impact

The function UnstakeAll don't unstake all the unstakable deposits.

Tools Used

Echidna

Recommendations

You can just add a if statement that make sure that the function unstake the deposit of the current epoch.

if(currentEpoch != epoch){
if (dr.epoch == 0 || currentEpoch - epoch <= lockCycle) continue;
}
Updates

Lead Judging Commences

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

Appeal created

0xphantom 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.