Fjord Token Staking

Relevant Github Links

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

Summary

• FjordStaking::_unstakeVested is an internal function, which can be called via unstakeVested and onStreamCanceled function.

• unstakeVested function allows users to unstake their whole NFT, while onStreamCanceled is called when the stream creator cancels the stream and the function either unstakes the NFT or removes the staked amount for the NFT.

• But when onStreamCanceled function calls _unstakeVested, where _unstakeVested function will unstake the amount for the NFT and also updates the staked amount on FjordPoints contract via FjordPoints::onUnstaked.

• But instead of passing the sablier NFT owner it passes msg.sender i.e., Sablier contract to the onUnstaked function and as Sablier contract has 0 staked amount on fjord points, it will result in a revert which thus reverts the onStreamCanceled function but Sablier contract will ignore this revert.

• Thus the stream was cancelled but staked data remains unupdated on FjordStaking contract and the sablier NFT owner will be able to receive rewards on the whole value including the value which was supposed to be unstaked.

Vulnerability Details

• The vulnerability is present in the FjordStaking::_unstakeVested where it passes msg.sender as user for which unstaking is done to onUnstaked function instead of streamOwner.

• Here, msg.sender will not be the sablier NFT owner (stream owner) in all cases.

• For the case when _unstakeVested is called by onStreamCanceled function, then msg.sender is going to be the Sablier contract and not the stream owner as a result of which it tries to reduce the staked amount data of Sablier contract on FjordPoints contract, thus leading to revert.

• This revert would thus cause the onStreamCanceled function to revert and all the updations related to unstaking of amount of user will be undone, but the stream creator cancelling of the stream would be successful as the revert caused by onStreamCanceled is ignored by Sablier contract.

Impact

• When stream is cancelled by creator, the NFT holder's will have no ownership of the unstreamed amount. But as onStreamCanceled faced a revert, the unstreamed amount is still in the accounting of the FjordStaking and FjordPoints contract.

• Thus, user will be able to receive rewards on the amount which they do not own as it is still in their staked amount.

Tools Used

Manual Review

Recommendations

In FjordStaking::_unstakeVested, instead of passing msg.sender to onUnstaked function, pass streamOwner