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

Centralisation Risk : addresses inside the mapping `authorizedSablierSenders` can cause the vFJO-Stakers a lose of funds

benterkiii

Vulnerability Details

as we know the streams after certen amount of time of being created the stream funds start being withdrawable.

the `stakeVested()` function allow users to stake their streams even if the withdrawable amount is positive, and they cant withdraw after stking the stream.
paste the folowing test contract inside the following directory:
2024-08-FJORD/test/unit/TestStakeVested.t.sol
and run forge test --mt testNftsWithPositiveWithdrawableAmountCanBeSTaked -vvv

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity =0.8.21;
​
import "forge-std/console.sol";
import "../FjordStakingBase.t.sol";
​
contract TestStakeVested is FjordStakingBase {
​
function testNftsWithPositiveWithdrawableAmountCanBeSTaked() public {
//1.alice have a 100 ether stream, alice can withdraw most of the stream funds.
uint256 streamID = createStream(token, true);
skip(9 days);
​
console.log("alice balance before staking:", token.balanceOf(alice));
​
//2.alice decide to stake the stream without withdrawing any funds.
vm.startPrank(alice);
SABLIER.approve(address(fjordStaking), streamID);
fjordStaking.stakeVested(streamID);
vm.stopPrank();
​
console.log("alice balance after staking:", token.balanceOf(alice));
​
NFTData memory nftData = fjordStaking.getStreamData(alice, streamID);
​
console.log("nft amount:", nftData.amount);
}
}

as we can se alice balance didnt change and the NFT get staked with all the funds.

inside the `SablierV2Lockup.sol` there is a public withdraw function that can be called by any authorizedSablierSenders and it will not revert when the to parameter is the stream recipient.
here a link to the withdraw function: https://github.com/johnpaulcas/v2-core/blob/d1157b49ed4bceeff0c4e437c9f723e88c134d3a/src/abstracts/SablierV2Lockup.sol#L220C4-L257C6

the authorizedSablierSenders can withdraw the funds to address(FjordStaking)with no chance to the user to get his funds back.


Impact

authorizedSablierSenders can cause a lose of funds to the user who prefer to stake their streams without withdrawing the `SablierV2Lockup.withdrawableAmountOf()` these streams.

POC

here is a test as a proof, copy paste the following test inside the file we opened early:

2024-08-FJORD/test/unit/TestStakeVested.t.sol

function testAuthorizedSenderCanWithdrawNftFundsAfterStaking() public {
//1.alice have a 100 ether stream, alice can withdraw most of the stream funds.
uint256 streamID = createStream(token, true);
skip(9 days);
​
console.log("the withdrawable amount before:", SABLIER.withdrawableAmountOf(streamID));
//2.alice decide to stake the stream without withdrawing any funds.
//the stream recipien will be address(fjordStaking).
vm.startPrank(alice);
SABLIER.approve(address(fjordStaking), streamID);
fjordStaking.stakeVested(streamID);
vm.stopPrank();
​
assert(fjordStaking.getStreamOwner(streamID) == alice);
assert(SABLIER.getRecipient(streamID) == address(fjordStaking));
​
//3.then an authorizedSablierSender want to make alice lose her funds,
//so he calls the SablierV2Lockup.withdraw public function and pass address(fjordStaking) as parameter.
console.log("fjordStaking balance before withdrawal:", token.balanceOf(address(fjordStaking)));
​
vm.startPrank(authorizedSender);
uint128 withdrawnAmt = SABLIER.withdrawableAmountOf(streamID);
SABLIER.withdraw(streamID, address(fjordStaking), withdrawnAmt);
vm.stopPrank();
​
console.log("alice loses this amount:", withdrawnAmt);
console.log("fjordStaking balance after withdrawal:", token.balanceOf(address(fjordStaking)));
​
//4.now when alice unstake her nft the withdrawable amount will be losed (withdrawn to the address(fjordStaking)).
vm.startPrank(alice);
fjordStaking.unstakeVested(streamID);
vm.stopPrank();
console.log("the withdrawable amount after:", SABLIER.withdrawableAmountOf(streamID));
}

Tools Used

manual review, foundry test

Recommendations

i sugest modifying the function to withdraw the stream funds to the user before staking
then stake the vFJO and the withdrawn FJO at the same time.
or revert when the `SablierV2Lockup.withdrawableAmountOf()` of the stream is positive and tell the users to withdraw then stake.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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