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

`onStreamCanceled` will revert because of wrong msg.sender calling `onUnstaked` in `FjordPoints.sol`

AllTooWell

Summary

onStreamCanceled will revert because of wrong msg.sender calling onUnstaked in FjordPoints.sol.

Vulnerability Details

If a SablierSender want to cancel a vested streamId, the flow is as follows:
cancel(Sablier) → _cancel(Sablier) → onStreamCanceled(FjordStaking) → _redeem(FjordStaking) → _unstakeVested(FjordStaking)

FjordStaking::_unstakeVested

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

onUnstaked is called with msg.sender as input. But in cancel flow, msg.sender is Sablier address. So onUnstaked will be revert because Sablier address has no stake on FjordStaking.

Impact

The cancel of streamId can't change the stake state of streamId. The stake amount of streamId is wrong and the user will get more reward.

POC

Paste code below in test/intergration folder.

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity =0.8.21;
​
import { FjordStaking } from "../../src/FjordStaking.sol";
import { FjordPoints } from "../../src/FjordPoints.sol";
import { FjordToken } from "../../src/FjordToken.sol";
​
import { ISablierV2LockupLinear } from "lib/v2-core/src/interfaces/ISablierV2LockupLinear.sol";
import { ISablierV2Lockup } from "lib/v2-core/src/interfaces/ISablierV2Lockup.sol";
import { Broker, LockupLinear } from "lib/v2-core/src/types/DataTypes.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { ud60x18 } from "@prb/math/src/UD60x18.sol";
​
import { Test } from "forge-std/Test.sol";
​
contract HookRevert is Test {
FjordStaking stake;
FjordPoints points;
FjordToken fjo;
​
uint256 streamId;
​
address deployer = makeAddr("deployer");
address rewardAdmin = makeAddr("rewardAdmin");
address sablierSender = makeAddr("sablierSender");
address user = makeAddr("user");
​
address internal constant SABLIER_ADDRESS = address(0xB10daee1FCF62243aE27776D7a92D39dC8740f95);
​
ISablierV2LockupLinear SABLIER = ISablierV2LockupLinear(SABLIER_ADDRESS);
​
function setUp() public {
uint256 mainnetFork = vm.createFork(vm.envString("ETHEREUM_RPC"), 18258796);
vm.selectFork(mainnetFork);
vm.startPrank(deployer);
fjo = new FjordToken();
points = new FjordPoints();
stake = new FjordStaking(
address(fjo), rewardAdmin, SABLIER_ADDRESS, sablierSender, address(points)
);
points.setStakingContract(address(stake));
vm.stopPrank();
streamId = createStream(sablierSender, user, IERC20(address(fjo)), true, 1 ether);
}
​
function test_hookRevert() public {
// user stake streamId
vm.startPrank(user);
vm.warp(block.timestamp + 5 days);
SABLIER.approve(address(stake), streamId);
stake.stakeVested(streamId);
vm.stopPrank();
​
// sablierSender cancel streamId
vm.startPrank(sablierSender);
ISablierV2Lockup(SABLIER_ADDRESS).cancel(streamId);
vm.stopPrank();
}
​
function createStream(
address sender,
address recipient,
IERC20 asset,
bool isCancelable,
uint256 amount
) internal returns (uint256 streamID) {
deal(address(asset), sender, amount);
vm.prank(sender);
asset.approve(address(SABLIER), amount);
​
LockupLinear.CreateWithRange memory params;
​
params.sender = sender;
params.recipient = recipient;
params.totalAmount = uint128(amount);
params.asset = IERC20(address(asset));
params.cancelable = isCancelable;
params.range = LockupLinear.Range({
start: uint40(block.timestamp + 1 days),
cliff: uint40(block.timestamp + 1 days),
end: uint40(block.timestamp + 11 days)
});
params.broker = Broker(address(0), ud60x18(0));
vm.prank(sender);
streamID = SABLIER.createWithRange(params);
}
}

forge test --mt test_hookRevert -vvvv

Logs:

├─ [79614] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::cancel(58)
│ ├─ [29606] FjordToken::transfer(sablierSender: [0xB9D6e253E40B396bA13B0f59B7816192708803c9], 600000000000000000 [6e17])
│ │ ├─ emit Transfer(from: 0xB10daee1FCF62243aE27776D7a92D39dC8740f95, to: sablierSender: [0xB9D6e253E40B396bA13B0f59B7816192708803c9], value: 600000000000000000 [6e17])
│ │ └─ ← [Return] true
│ ├─ [15460] FjordStaking::onStreamCanceled(58, sablierSender: [0xB9D6e253E40B396bA13B0f59B7816192708803c9], 600000000000000000 [6e17], 400000000000000000 [4e17])
│ │ ├─ [8365] FjordPoints::onUnstaked(0xB10daee1FCF62243aE27776D7a92D39dC8740f95, 600000000000000000 [6e17])
│ │ │ └─ ← [Revert] UnstakingAmountExceedsStakedAmount()
│ │ └─ ← [Revert] UnstakingAmountExceedsStakedAmount()
│ ├─ emit CancelLockupStream(streamId: 58, sender: sablierSender: [0xB9D6e253E40B396bA13B0f59B7816192708803c9], recipient: FjordStaking: [0xfF2Bd636B9Fc89645C2D336aeaDE2E4AbaFe1eA5], senderAmount: 600000000000000000 [6e17], recipientAmount: 400000000000000000 [4e17])
│ ├─ emit MetadataUpdate(: 58)
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
└─ ← [Stop]

Tools Used

manual and foundry

Recommendations

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 about 1 year 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.