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

Incorrect implementation of _unstakeVested function in FjordStaking contract allows continued reward accrual after stream cancellation

ast3ros

Summary

The _unstakeVested function in the FjordStaking contract incorrectly uses msg.sender instead of streamOwner when calling points.onUnstaked(). This error leads to the inability to properly reduce staked amounts when Sablier streams are canceled, allowing stream owners to continue accruing rewards even after their streams have been cancelled.

Vulnerability Details

When unstake vested, the contract calls the onUnstaked function of the FjordPoints contract with the msg.sender address to reduce the staked amount recorded in the FjordPoints contract.

function _unstakeVested(address streamOwner, uint256 _streamID, uint256 amount) internal {
...
​
points.onUnstaked(msg.sender, amount); // @audit should be streamOwner instead of msg.sender
​
emit VestedUnstaked(streamOwner, epoch, amount, _streamID);
}

https://github.com/Cyfrin/2024-08-fjord/blob/14cab810598ddda6008d9523d0ed4a428b1b1153/src/FjordStaking.sol#L561

This function is called by onStreamCanceled when a Sablier stream is canceled.

The use of msg.sender in _unstakeVested causes the following issues:

  • When a stream is canceled, the call to points.onUnstaked() fails because it's attempting to reduce the staked amount for the Sablier contract (msg.sender) rather than the actual stream owner.

  • Due to this failure, the stream owner's staked amount in FjordPoints remains unchanged.

  • Consequently, the stream owner continues to accrue rewards based on their original staked amount, even after their stream has been canceled.

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

https://github.com/Cyfrin/2024-08-fjord/blob/14cab810598ddda6008d9523d0ed4a428b1b1153/src/FjordStaking.sol#L823-L844

It means that the stakedAmount of the stream owner is not reduced when the stream is canceled. And even though the stream is canceled, the stream owner can still earn Fjord point rewards as if the stream is still active.

Impact

  • Stream owners can continue to earn rewards without maintaining the required stake, leading to an unfair distribution of rewards

  • Stream senders could exploit this vulnerability by repeatedly creating and canceling streams to artificially inflate their reward accrual.

Tools Used

Manual

Recommendations

Modify the _unstakeVested function to use the streamOwner parameter instead of msg.sender:

function _unstakeVested(address streamOwner, uint256 _streamID, uint256 amount) internal {
...
​
- points.onUnstaked(msg.sender, amount);
+ points.onUnstaked(streamOwner, amount);
​
emit VestedUnstaked(streamOwner, epoch, amount, _streamID);
}
Updates

Lead Judging Commences

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

Sablier calls `onStreamCanceled` > `_unstakeVested` > points.onUnstaked(msg.sender, amount); making the effects of points.onUnstaked apply to sablier who's the msg.sender.

Indeed the `points.onUnstaked` should use the streamOwner instead of msg.sender as an input parameter. Impact: high - The vested stakers who got their streams canceled will keep on receiving rewards (points included) for the previously staked stream. Likelihood: low - whenever a Sablier stream sender decides to `cancel()` a recipient's stream

Support

FAQs

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