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

Lock cycle completion criterium is not checked in the `onStreamCanceled()` hook of Sablier when canceling a stream

M M

Description

The vested staking method, which is one of the staking modes in the FjordStaking contract,
is a process where a user's tokens are gradually released over a set period via the Sablier reward streams. A Sablier stream is recognized by its own stream ID
which contains the details of the vested staking of a user.

Now, in the case of a decision to unsubscribe to such streams, a user can call unstakeVested() function. This function is responsible for unstaking
vested FJORD tokens from the contract:

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);
}

An important check here is that, in the case of a discrepancy between the currentEpoch and a stream's epoch variable, the contract must ensure that the
mentioned difference exceeds the lock cycle completion and not allow for an early unstake. This check ensures that the lock cycle has been completed and it is
safe to unstake.

However, if we look at the onStreamCanceled(), we cannot see such a check:

function onStreamCanceled(
uint256 streamId,
address sender,
uint128 senderAmount,
uint128 /*recipientAmount*/
) external override onlySablier checkEpochRollover {
address streamOwner = _streamIDOwners[streamId];
​
if (streamOwner == address(0)) revert StreamOwnerNotFound();
​
_redeem(streamOwner);
​
NFTData memory nftData = _streamIDs[streamOwner][streamId];
​
uint256 amount =
uint256(senderAmount) > nftData.amount ? nftData.amount : uint256(senderAmount);
​
_unstakeVested(streamOwner, streamId, amount);
​
emit SablierCanceled(streamOwner, streamId, sender, amount);
}

This means that canceling a Sablier stream doesn't account for an early unstake, and directly calls _unstakeVested().

Impact

Canceling a Sablier stream lets for an early stake, breaking the cycle completion check invariant

Tools Used

Manual review

Recommended Mitigation

function onStreamCanceled(
uint256 streamId,
address sender,
uint128 senderAmount,
uint128 /*recipientAmount*/
) external override onlySablier checkEpochRollover {
address streamOwner = _streamIDOwners[streamId];
​
if (streamOwner == address(0)) revert StreamOwnerNotFound();
​
_redeem(streamOwner);
​
NFTData memory nftData = _streamIDs[streamOwner][streamId];
​
uint256 amount =
uint256(senderAmount) > nftData.amount ? nftData.amount : uint256(senderAmount);
​
+ if (currentEpoch != nftData.epoch) {
+ // If epoch less than current epoch then user can unstake after at complete lockCycle
+ if (currentEpoch - nftData.epoch <= lockCycle) revert UnstakeEarly();
+ }
​
_unstakeVested(streamOwner, streamId, amount);
​
emit SablierCanceled(streamOwner, streamId, sender, amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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