Fjord Token Staking

Summary

When a stream sender of a FjordToken stream in SablierV2LockUp cancels a stream, the nonclaimable but VestedStaked FjordToken tokens will be refunded to the stream sender. The lack of implementattion of the supportInterface function in the FjordStaking contract makes notifying the FjordStaking contract through FjordStaking::onStreamCanceled to unstake these withdrawn tokens totally impossible.

Vulnerability Details

Stream recipients can stake tokens vested in a SablierV2LockUp contracts, and then transfer the recipient rights of the stream to the address of FjordStaking contract, to ensure that no more tokens are withdrawn from this stream by the recipient.

Code from FjordStaking::StakeVested Line 435

Issues arise when the stream sender decides, to cancel the stream. During this cancellation, the function FjordStaking::onStreamCanceled is supposed to be called to ensure that the withdrawn deposited amount is deducted so that it stops accuring rewards for the stream owner/reciptient.

But in the logic of SablierV2LockUp::_cancel(streamId), for the function FjordStaking::onStreamCanceled to called by the SablierV2LockUp contract, the stream recipient, in this case the FjordStaking contract, has to be _allowedToHook[recipient].

This leads to two issues:
The address of FjordStaking contract has to be allowed to hook in FjordStaking::allowToHook(address recipient) but this is an adminOnly function where the admin of the sablier contract may refuse to call this functiom on the FjordStaking contract address.

Another issue which makes it totally impossible for the FjordStaking::onStreamCanceled to be called is the fact that even if the admin of the sablier contract decides to do so, he cannot. Allow me to explain.

According to the sablier documentation a recipient must:-

Link to docs

In SablierV2LockUp::allowToHook(address recipient) function a check call is made to the recipient contract to see if it implements the type(ISablierLockupRecipient).interfaceId supportsInterface which the FjordStaking contract does not.

SablierV2LockUp Line 264

As you can see, it reverts, and the admin cannot allow the contract to hook. The check in SablierV2LockUp::_cancel(streamId) can't be true and the FjordStaking::onStreamCanceled will never be called.

SablierV2LockUp line 596

There is nowhere in the readme of this contest where it says that stream senders will be trusted, they only have to be authorised.

Impact

The recipient of the stream will continue earning rewards forever for tokens that have already been withdrawn, hence a loss of rewards by the contract.

Tools Used

Manual Review

Recommendation

Consider implementing the support interface required by the Sablier contract so that you can be allowed to hook and notified when such withdraws are made.

Make sure the FjordStaking contract is allowed to hook.