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

Unstaking of previous stakes in the same epoch that a recent stake happened can Cause Loss of funds

M M

Description

The fjordStaking contract lets users stake their Fjord tokens at the current epoch and also unstake their past stakes. The problem arises in the unstaking part
of the code that doesn't consider the staking and unstaking (the previous ones) at the same epoch:

function unstake(uint16 _epoch, uint256 _amount)
external
checkEpochRollover
redeemPendingRewards
returns (uint256 total)
{
if (_amount == 0) revert InvalidAmount();
​
DepositReceipt storage dr = deposits[msg.sender][_epoch];
​
...
​
if (dr.staked == 0 && dr.vestedStaked == 0) {
// no longer a valid unredeemed epoch
@> if (userData[msg.sender].unredeemedEpoch == currentEpoch) {
@> userData[msg.sender].unredeemedEpoch = 0;
}
delete deposits[msg.sender][_epoch];
_activeDeposits[msg.sender].remove(_epoch);
}
​
...
}

The function checks for unredeemedEpoch, and if it's equal to currentEpoch, sets the unredeemed epoch as 0.
unredeemedEpoch is used in _redeem, so if set it to 0, the current staked amount will not added to users stake amount.

Impact

Loss of rewards and stakes for users if they unstake their previously deposited assets exactly at the same epoch when they newly invested

Proof of Concept

A possible scenario for this issue can be in such a way:

  1. Alice deposits 1e20 at the epoch 4

  2. After 8 weeks, she makes another 1e20 staking. (ex: epoch 12)

  3. She decides to unstake her stake for epoch 4 at epoch 12.

  4. This makes the unredeemed epoch of Alice become 0.

  5. so now _redeem function doesn't include the second 1e20 to his stake amount.

  6. After some time (e.g. 7 weeks) she can't even unstake because the code doesn't show any positive deposit for Alice

This test shows the unredeemed epoch of Alice becomes zero, in the case of staking and unstaking a previous epoch at the same epoch:

function testUnredeemedEpoch() public {
deal(address(token), alice, 1e25);
vm.startPrank(alice);
token.approve(address(fjordStaking), type(uint256).max);
​
vm.warp(block.timestamp + 3 weeks); // 3 weeks from the start
fjordStaking.stake(1e20);
vm.warp(block.timestamp + 8 weeks); // Now we are at epoch 12
fjordStaking.stake(1e20);
​
// UNSTAKE EPOCH 4
fjordStaking.unstake(4, 1e20);
​
(,,uint16 unredeemedEpoch,) = fjordStaking.userData(alice);
console.log("Unredeemed Epoch for Alice is: ", unredeemedEpoch);
​
vm.warp(block.timestamp + 7 weeks); // Let's check Alice can unstake after a period
// UNTILL NOW HE STAKES 1E20 + 1E20, UNSTKAE EPOCH 4 (1E20), SO HE MUST HAVE 1E20
(uint256 totalStaked,,,) = fjordStaking.userData(alice);
console.log("Alice's total stake is: ", totalStaked);
​
vm.expectRevert(); // It will revert here
fjordStaking.unstake(12, 1e20);
}

The test result is:

Ran 1 test for test/FjordTest.t.sol:FjordTest
[PASS] testUnredeemedEpoch() (gas: 735889)
Logs:
Unredeemed Epoch for Alice is: 0
Alice's total stake is: 0
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.88ms (2.00ms CPU time)

Tools Used

Manual review

Recommended Mitigation

- if (userData[msg.sender].unredeemedEpoch == currentEpoch) {
+ if (userData[msg.sender].unredeemedEpoch == currentEpoch && _epoch == currentEpoch) {

it must check that is user unstaking current epoch or not.

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`unredeemedEpoch` is set to zero due to a false if condition in `unstake` and `_unstakeVested` function

Users that try to unstake tokens from an earlier epoch after they staked in the current epoch will have their ` unredeemedEpoch` set to 0, leading to them being unable to access the newly staked tokens. Impact: High – Tokens are lost Likelihood: Medium – It happens every time a user performs the respective sequence of calls. It’s not always but it’s also not a low likelihood scenario. It’s normal usage of the protocol, that doesn’t necessarily require special conditions.

Support

FAQs

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