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

`unstakeVested` fn doesn't check if the current epoch is the same as the staked epoch before allowing unstaking

Vulnerability Details

function unstakeVested(uint256 _streamID) external checkEpochRollover redeemPendingRewards {
//CHECK
NFTData memory data = _streamIDs[msg.sender][_streamID];
DepositReceipt memory dr = deposits[msg.sender][data.epoch];
if (data.epoch == 0 || data.amount == 0 || dr.vestedStaked == 0 || dr.epoch == 0) {
revert DepositNotFound();
}
// If epoch is same as current epoch then user can unstake immediately
if (currentEpoch != data.epoch) {
// If epoch less than current epoch then user can unstake after at complete lockCycle
if (currentEpoch - data.epoch <= lockCycle) revert UnstakeEarly();
}
_unstakeVested(msg.sender, _streamID, data.amount);
}
/// @notice Partial or fully unstake vested .
function _unstakeVested(address streamOwner, uint256 _streamID, uint256 amount) internal {
NFTData storage data = _streamIDs[streamOwner][_streamID];
DepositReceipt storage dr = deposits[streamOwner][data.epoch];
if (amount > data.amount) revert InvalidAmount();
bool isFullUnstaked = data.amount == amount;
uint16 epoch = data.epoch;
dr.vestedStaked -= amount;
if (currentEpoch != data.epoch) {
totalStaked -= amount;
totalVestedStaked -= amount;
userData[streamOwner].totalStaked -= amount;
} else {
// unstake immediately
newStaked -= amount;
newVestedStaked -= amount;
}
if (dr.vestedStaked == 0 && dr.staked == 0) {
// instant unstake
if (userData[streamOwner].unredeemedEpoch == currentEpoch) {
userData[streamOwner].unredeemedEpoch = 0;
}
delete deposits[streamOwner][data.epoch];
_activeDeposits[streamOwner].remove(data.epoch);
}
// fully unstake
if (isFullUnstaked) {
delete _streamIDs[streamOwner][_streamID];
delete _streamIDOwners[_streamID];
} else {
data.amount -= amount;
}
//INTERACT
if (isFullUnstaked) {
sablier.transferFrom({ from: address(this), to: streamOwner, tokenId: _streamID });
}
points.onUnstaked(msg.sender, amount);
emit VestedUnstaked(streamOwner, epoch, amount, _streamID);
}

in the unstakeVested function in FjordStaking.sol,

The function doesn't check if the current epoch is the same as the staked epoch before allowing unstaking. This means users could potentially unstake vested tokens immediately after staking them, bypassing the intended 6-week (6 epoch) lock period.

The function checks:

if (currentEpoch != data.epoch) {
if (currentEpoch - data.epoch <= lockCycle) revert UnstakeEarly();
}

But it should also prevent unstaking if the current epoch is the same as the staked epoch. A correct check would be:

if (currentEpoch <= data.epoch || (currentEpoch - data.epoch <= lockCycle)) revert UnstakeEarly();

This bug could allow users to stake and immediately unstake vested tokens, potentially exploiting the system.

Impact

  1. User stakes vested tokens using stakeVested(streamID).

  2. In the same transaction or epoch, user calls unstakeVested(streamID).

  3. The tokens are immediately unstaked without waiting for the lock period.

This could be exploited to:

  1. Gain unfair advantages in reward distribution.

  2. Manipulate the total staked amount to affect reward calculations.

To fix, add a check to ensure currentEpoch > data.epoch before allowing unstaking.

Tools Used

Manual review

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.