Submission Details
Severity:
`onStreamCanceled` will never successfully unstake the `streamOwner`'s vested stake amount.
Summary
OnStreamCanceled will never successfully unstake the user's vested stake amount because the _unstakeVested() function is flawed
Vulnerability Details
Users can stake their Sablier stream NFTs into the FjordStaking contract to earn rewards and points. For this to happen, the stream must be "warm" (active), and the stream's sender must be authorized by the staking contract. A Sablier stream can be canceled by the sender at any time. When a stream is canceled, Sablier triggers a callback to the recipient if the recipient is a contract, using the following logic:
if (msg.sender == sender) {
if (recipient.code.length > 0) {
try ISablierV2LockupRecipient(recipient).onStreamCanceled({
streamId: streamId,
sender: sender,
senderAmount: senderAmount,
recipientAmount: recipientAmount
}) { } catch { }
}
}
When a user stakes their stream NFT to the staking contract, the recipient of the stream is updated to the staking contract. Therefore, whoever holds the stream NFT becomes the recipient. As a result, the staking contract implements the onStreamCanceled() function, which is structured as follows:
function onStreamCanceled(
uint256 streamId,
address sender,
uint128 senderAmount,
uint128 /*recipientAmount*/
) external override onlySablier checkEpochRollover {
address streamOwner = _streamIDOwners[streamId];
if (streamOwner == address(0)) revert StreamOwnerNotFound();
_redeem(streamOwner);
NFTData memory nftData = _streamIDs[streamOwner][streamId];
uint256 amount = uint256(senderAmount) > nftData.amount ? nftData.amount : uint256(senderAmount);
_unstakeVested(streamOwner, streamId, amount);
emit SablierCanceled(streamOwner, streamId, sender, amount);
}
The onStreamCanceled() function is access-controlled so that only the Sablier contract can invoke it, as enforced by the following modifier:
modifier onlySablier() {
if (msg.sender != address(sablier)) revert CallerDisallowed();
_;
}
However, there is a critical flaw in the _unstakeVested function:
function _unstakeVested(address streamOwner, uint256 _streamID, uint256 amount) internal {
NFTData storage data = _streamIDs[streamOwner][_streamID];
DepositReceipt storage dr = deposits[streamOwner][data.epoch];
if (amount > data.amount) revert InvalidAmount();
bool isFullUnstaked = data.amount == amount;
uint16 epoch = data.epoch;
dr.vestedStaked -= amount;
if (currentEpoch != data.epoch) {
totalStaked -= amount;
totalVestedStaked -= amount;
userData[streamOwner].totalStaked -= amount;
} else {
// Unstake immediately
newStaked -= amount;
newVestedStaked -= amount;
}
if (dr.vestedStaked == 0 && dr.staked == 0) {
// Instant unstake
if (userData[streamOwner].unredeemedEpoch == currentEpoch) {
userData[streamOwner].unredeemedEpoch = 0;
}
delete deposits[streamOwner][data.epoch];
_activeDeposits[streamOwner].remove(data.epoch);
}
// Fully unstake
if (isFullUnstaked) {
delete _streamIDs[streamOwner][_streamID];
delete _streamIDOwners[_streamID];
} else {
data.amount -= amount;
}
// INTERACT
if (isFullUnstaked) {
sablier.transferFrom({ from: address(this), to: streamOwner, tokenId: _streamID });
}
points.onUnstaked(msg.sender, amount);
emit VestedUnstaked(streamOwner, epoch, amount, _streamID);
}
This function attempts to unstake the stream owner’s amount, which is equivalent to the amount received by the stream sender upon cancellation, from the stream owner’s stake in the corresponding epoch. It then calls points.onUnstaked(msg.sender, amount); to calculate the user's points and add them to the unclaimed rewards, while reducing the user's stake in the FjordPoints contract.
However, the points.onUnstaked(msg.sender, amount); call will always fail because the function's first input, msg.sender, should be the stream owner. Since _unstakeVested is called within the onStreamCanceled() function, which can only be invoked by the Sablier contract, msg.sender will be the Sablier contract. As the Sablier contract has no stake in the FjordPoints contract, the transaction will revert with an UnstakingAmountExceedsStakedAmount() error:
if (amount > userInfo.stakedAmount) {
revert UnstakingAmountExceedsStakedAmount();
}
POC :
// SPDX-License-Identifier: AGPL-3.0-only
import "../src/FjordStaking.sol";
import {FjordPoints} from "../src/FjordPoints.sol";
import {Test} from "forge-std/Test.sol";
import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol";
import {FjordPointsMock} from "./mocks/FjordPointsMock.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 "lib/v2-core/src/libraries/Errors.sol";
contract cancelMsgSender is Test {
address Bob = makeAddr("Bob");
address sender = makeAddr("sender");
uint256 public startTime;
uint256 public endTime = 70 days;
MockERC20 token;
address minter = makeAddr("minter");
address authorizedSender = address(this);
address internal constant SABLIER_ADDRESS =
address(0xB10daee1FCF62243aE27776D7a92D39dC8740f95);
ISablierV2LockupLinear SABLIER = ISablierV2LockupLinear(SABLIER_ADDRESS);
FjordStaking fjordStaking;
uint256 public streamID;
function setUp() public {
address points = address(new FjordPoints());
token = new MockERC20("Fjord", "FJO", 18);
uint256 amount = 10 ether;
deal(address(token), sender, amount);
fjordStaking = new FjordStaking(
address(token),
minter,
SABLIER_ADDRESS,
sender,
points
);
vm.startPrank(FjordPoints(points).owner());
FjordPoints(points).setStakingContract(address(fjordStaking));
vm.stopPrank();
vm.prank(sender);
token.approve(address(SABLIER), amount);
LockupLinear.CreateWithRange memory params;
params.sender = sender;
params.recipient = Bob;
params.totalAmount = uint128(amount);
params.asset = IERC20(address(token));
params.cancelable = true;
params.range = LockupLinear.Range({
start: uint40(block.timestamp),
cliff: uint40(block.timestamp + 2 days),
end: uint40(block.timestamp + endTime)
});
params.broker = Broker(address(0), ud60x18(0));
vm.prank(sender);
streamID = SABLIER.createWithRange(params);
vm.startPrank(Bob);
SABLIER.approve(address(fjordStaking), streamID);
fjordStaking.stakeVested(streamID);
vm.stopPrank();
vm.warp(block.timestamp + 30 days);
}
function testCancelStream() public {
vm.startPrank(sender);
SABLIER.cancel(streamID);
vm.stopPrank();
vm.startPrank(address(SABLIER));
fjordStaking.onStreamCanceled(
streamID,
sender,
uint128(token.balanceOf(sender)),
uint128(10 ether)
);
vm.stopPrank();
}
}
Logs :
Ran 1 test for test/canelSender.t.sol:cancelMsgSender
[FAIL. Reason: UnstakingAmountExceedsStakedAmount()] testCancelStream() (gas: 552677)
Traces:
[5677290] cancelMsgSender::setUp()
├─ [1206823] → new FjordPoints@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
│ └─ ← [Return] 5471 bytes of code
├─ [702855] → new MockERC20@0x2e234DAe75C793f67A35089C9d99245E1C58470b
│ └─ ← [Return] 3276 bytes of code
├─ [2542] MockERC20::balanceOf(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681]) [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::record()
│ └─ ← [Return]
├─ [542] MockERC20::balanceOf(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681]) [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::accesses(MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b])
│ └─ ← [Return] [0xcc3c5569c22f500bcbd5e829c941fefe7b077550524cad163f78fcc868e35be2], []
├─ [0] VM::load(MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xcc3c5569c22f500bcbd5e829c941fefe7b077550524cad163f78fcc868e35be2) [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000000000000000000000000000
├─ emit WARNING_UninitedSlot(who: MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], slot: 92378421434556971428010548848190553934854352934444475204177128314736394591202 [9.237e76])
├─ [0] VM::load(MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xcc3c5569c22f500bcbd5e829c941fefe7b077550524cad163f78fcc868e35be2) [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000000000000000000000000000
├─ [542] MockERC20::balanceOf(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681]) [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::store(MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xcc3c5569c22f500bcbd5e829c941fefe7b077550524cad163f78fcc868e35be2, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
│ └─ ← [Return]
├─ [542] MockERC20::balanceOf(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681]) [staticcall]
│ └─ ← [Return] 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]
├─ [0] VM::store(MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xcc3c5569c22f500bcbd5e829c941fefe7b077550524cad163f78fcc868e35be2, 0x0000000000000000000000000000000000000000000000000000000000000000)
│ └─ ← [Return]
├─ emit SlotFound(who: MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], fsig: 0x70a08231, keysHash: 0x0011e69f40fa71424576c6c7187be39af85bd6213f80bf888a0abab41399ac10, slot: 92378421434556971428010548848190553934854352934444475204177128314736394591202 [9.237e76])
├─ [0] VM::load(MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xcc3c5569c22f500bcbd5e829c941fefe7b077550524cad163f78fcc868e35be2) [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000000000000000000000000000
├─ [0] VM::store(MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xcc3c5569c22f500bcbd5e829c941fefe7b077550524cad163f78fcc868e35be2, 0x0000000000000000000000000000000000000000000000008ac7230489e80000)
│ └─ ← [Return]
├─ [542] MockERC20::balanceOf(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681]) [staticcall]
│ └─ ← [Return] 10000000000000000000 [1e19]
├─ [2756964] → new FjordStaking@0xF62849F9A0B5Bf2913b396098F7c7019b51A820a
│ └─ ← [Return] 13101 bytes of code
├─ [404] FjordPoints::owner() [staticcall]
│ └─ ← [Return] cancelMsgSender: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]
├─ [0] VM::startPrank(cancelMsgSender: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return]
├─ [22742] FjordPoints::setStakingContract(FjordStaking: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a])
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::prank(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681])
│ └─ ← [Return]
├─ [24546] MockERC20::approve(0xB10daee1FCF62243aE27776D7a92D39dC8740f95, 10000000000000000000 [1e19])
│ ├─ emit Approval(owner: sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], spender: 0xB10daee1FCF62243aE27776D7a92D39dC8740f95, value: 10000000000000000000 [1e19])
│ └─ ← [Return] true
├─ [0] VM::prank(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681])
│ └─ ← [Return]
├─ [166919] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::createWithRange(CreateWithRange({ sender: 0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681, recipient: 0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C, totalAmount: 10000000000000000000 [1e19], asset: 0x2e234DAe75C793f67A35089C9d99245E1C58470b, cancelable: true, range: Range({ start: 1724688527 [1.724e9], cliff: 1724861327 [1.724e9], end: 1730736527 [1.73e9] }), broker: Broker({ account: 0x0000000000000000000000000000000000000000, fee: 0 }) }))
│ ├─ [2538] 0xC3Be6BffAeab7B297c03383B4254aa3Af2b9a5BA::protocolFees(MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]) [staticcall]
│ │ └─ ← [Return] 0
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: Bob: [0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C], tokenId: 1408)
│ ├─ [25581] MockERC20::transferFrom(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], 0xB10daee1FCF62243aE27776D7a92D39dC8740f95, 10000000000000000000 [1e19])
│ │ ├─ emit Transfer(from: sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], to: 0xB10daee1FCF62243aE27776D7a92D39dC8740f95, value: 10000000000000000000 [1e19])
│ │ └─ ← [Return] true
│ ├─ emit CreateLockupLinearStream(streamId: 1408, funder: sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], sender: sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], recipient: Bob: [0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C], amounts: CreateAmounts({ deposit: 10000000000000000000 [1e19], protocolFee: 0, brokerFee: 0 }), asset: MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], cancelable: true, range: Range({ start: 1724688527 [1.724e9], cliff: 1724861327 [1.724e9], end: 1730736527 [1.73e9] }), broker: 0x0000000000000000000000000000000000000000)
│ └─ ← [Return] 1408
├─ [0] VM::startPrank(Bob: [0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C])
│ └─ ← [Return]
├─ [25000] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::approve(FjordStaking: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 1408)
│ ├─ emit Approval(owner: Bob: [0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C], approved: FjordStaking: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], tokenId: 1408)
│ └─ ← [Return]
├─ [355816] FjordStaking::stakeVested(1408)
│ ├─ [1170] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::isStream(1408) [staticcall]
│ │ └─ ← [Return] true
│ ├─ [2132] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::isCold(1408) [staticcall]
│ │ └─ ← [Return] false
│ ├─ [1389] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::getSender(1408) [staticcall]
│ │ └─ ← [Return] sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681]
│ ├─ [1637] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::getAsset(1408) [staticcall]
│ │ └─ ← [Return] MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]
│ ├─ [1307] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::getDepositedAmount(1408) [staticcall]
│ │ └─ ← [Return] 10000000000000000000 [1e19]
│ ├─ [1537] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::getWithdrawnAmount(1408) [staticcall]
│ │ └─ ← [Return] 0
│ ├─ [1527] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::getRefundedAmount(1408) [staticcall]
│ │ └─ ← [Return] 0
│ ├─ [29262] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::transferFrom(Bob: [0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C], FjordStaking: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 1408)
│ │ ├─ emit Transfer(from: Bob: [0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C], to: FjordStaking: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], tokenId: 1408)
│ │ └─ ← [Return]
│ ├─ [54148] FjordPoints::onStaked(Bob: [0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C], 10000000000000000000 [1e19])
│ │ ├─ emit Staked(user: Bob: [0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C], amount: 10000000000000000000 [1e19])
│ │ └─ ← [Stop]
│ ├─ emit VestedStaked(user: Bob: [0x4dBa461cA9342F4A6Cf942aBd7eacf8AE259108C], epoch: 1, streamID: 1408, amount: 10000000000000000000 [1e19])
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(1727280527 [1.727e9])
│ └─ ← [Return]
└─ ← [Stop]
[552677] cancelMsgSender::testCancelStream()
├─ [0] VM::startPrank(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681])
│ └─ ← [Return]
├─ [305269] 0xB10daee1FCF62243aE27776D7a92D39dC8740f95::cancel(1408)
│ ├─ [29652] MockERC20::transfer(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], 5714285714285714290 [5.714e18])
│ │ ├─ emit Transfer(from: 0xB10daee1FCF62243aE27776D7a92D39dC8740f95, to: sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], value: 5714285714285714290 [5.714e18])
│ │ └─ ← [Return] true
│ ├─ [228569] FjordStaking::onStreamCanceled(1408, sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], 5714285714285714290 [5.714e18], 4285714285714285710 [4.285e18])
│ │ ├─ emit RewardPerTokenChanged(epoch: 1, rewardPerToken: 0)
│ │ ├─ emit RewardPerTokenChanged(epoch: 2, rewardPerToken: 0)
│ │ ├─ emit RewardPerTokenChanged(epoch: 3, rewardPerToken: 0)
│ │ ├─ emit RewardPerTokenChanged(epoch: 4, rewardPerToken: 0)
│ │ ├─ [86553] FjordPoints::onUnstaked(0xB10daee1FCF62243aE27776D7a92D39dC8740f95, 5714285714285714290 [5.714e18])
│ │ │ ├─ emit PointsDistributed(points: 100000000000000000000 [1e20], pointsPerToken: 40000000000000000000 [4e19])
│ │ │ └─ ← [Revert] UnstakingAmountExceedsStakedAmount()
│ │ └─ ← [Revert] UnstakingAmountExceedsStakedAmount()
│ ├─ emit CancelLockupStream(streamId: 1408, sender: sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], recipient: FjordStaking: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], senderAmount: 5714285714285714290 [5.714e18], recipientAmount: 4285714285714285710 [4.285e18])
│ ├─ emit MetadataUpdate(: 1408)
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(0xB10daee1FCF62243aE27776D7a92D39dC8740f95)
│ └─ ← [Return]
├─ [542] MockERC20::balanceOf(sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681]) [staticcall]
│ └─ ← [Return] 5714285714285714290 [5.714e18]
├─ [228569] FjordStaking::onStreamCanceled(1408, sender: [0xCD1722F3947DEf4Cf144679Da39c4c32BDC35681], 5714285714285714290 [5.714e18], 10000000000000000000 [1e19])
│ ├─ emit RewardPerTokenChanged(epoch: 1, rewardPerToken: 0)
│ ├─ emit RewardPerTokenChanged(epoch: 2, rewardPerToken: 0)
│ ├─ emit RewardPerTokenChanged(epoch: 3, rewardPerToken: 0)
│ ├─ emit RewardPerTokenChanged(epoch: 4, rewardPerToken: 0)
│ ├─ [86553] FjordPoints::onUnstaked(0xB10daee1FCF62243aE27776D7a92D39dC8740f95, 5714285714285714290 [5.714e18])
│ │ ├─ emit PointsDistributed(points: 100000000000000000000 [1e20], pointsPerToken: 40000000000000000000 [4e19])
│ │ └─ ← [Revert] UnstakingAmountExceedsStakedAmount()
│ └─ ← [Revert] UnstakingAmountExceedsStakedAmount()
└─ ← [Revert] UnstakingAmountExceedsStakedAmount()
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 10.02s (9.20ms CPU time)
Ran 1 test suite in 19.95s (10.02s CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/canelSender.t.sol:cancelMsgSender
[FAIL. Reason: UnstakingAmountExceedsStakedAmount()] testCancelStream() (gas: 552677)
Encountered a total of 1 failing tests, 0 tests succeeded
Impact
Since the stream is canceled, the amount that the stream sender received should be unstaked from the stream owner's stake. However, due to the flawed code logic, this unstaking process fails. As a result, the stream owner continues to earn rewards for a stake that no longer exists
Tools Used
Manual
Recommendations
add the following code
points.onUnstaked(streamOwner, amount);
Updates
Lead Judging Commences
inallhonesty about 1 year ago
Submission Judgement Published 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
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
66230 USDC / LOC
16,000 USDC
2,500 USDC
1,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.