Summary
The SablierFlow contract contains an issue where approximately 46% of deposited funds can become permanently locked during normal stream operations. The issue stems from the interaction between withdrawal mechanisms, protocol fee calculations, and stream rate implementations, leaving no mechanism to recover the stuck funds.
Vulnerability Details
The vulnerability exists in the withdrawal and stream calculation logic of the SablierFlow contract. The primary issues are found in these functions:
This limits withdrawals to only the total debt amount
https://github.com/Cyfrin/2024-10-sablier/blob/963bf61b9d8ffe3eb06cbcf1c53f0ab88dbf0eb0/src/SablierFlow.sol#L473
The balance check condition that creates two different withdrawal paths
https://github.com/Cyfrin/2024-10-sablier/blob/963bf61b9d8ffe3eb06cbcf1c53f0ab88dbf0eb0/src/SablierFlow.sol#L468-L470
The withdrawal execution that relies on the problematic calculation
https://github.com/Cyfrin/2024-10-sablier/blob/963bf61b9d8ffe3eb06cbcf1c53f0ab88dbf0eb0/src/SablierFlow.sol#L431
If balance > totalDebt: Only totalDebt can be withdrawn
Remaining funds (balance - totalDebt) become permanently locked
No mechanism exists to recover these locked funds
function _withdraw(uint256 streamId, address to, uint128 amount) internal {
withdrawableAmount = _coveredDebtOf(streamId);
}
function _coveredDebtOf(uint256 streamId) internal view returns (uint128) {
uint128 balance = _streams[streamId].balance;
if (balance == 0) {
return 0;
}
uint256 totalDebt = _totalDebtOf(streamId);
if (balance < totalDebt) {
return balance;
}
return totalDebt.toUint128();
}
The issue arises from:
Incomplete withdrawal calculations
Protocol fee interactions with stream balance
No mechanism to recover stuck funds
Rounding issues in rate calculations
Impact
The vulnerability leads to permanent loss of user funds. Here's a proof of concept test demonstrating the issue:
pragma solidity ^0.8.22;
import { Test, console2 } from "forge-std/src/Test.sol";
import { SablierFlow } from "../src/SablierFlow.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { ERC20Mock } from "./mocks/ERC20Mock.sol";
import { ud60x18 } from "@prb/math/src/UD60x18.sol";
import { ud21x18, UD21x18 } from "@prb/math/src/UD21x18.sol";
import { IFlowNFTDescriptor } from "../src/interfaces/IFlowNFTDescriptor.sol";
contract TestStuckFunds is Test {
SablierFlow public sablier;
ERC20Mock public token;
address public admin = address(1);
address public alice = address(2);
address public bob = address(3);
function setUp() public {
sablier = new SablierFlow(admin, IFlowNFTDescriptor(address(0)));
token = new ERC20Mock("Test Token", "TEST", 18);
token.mint(alice, 1000e18);
vm.prank(alice);
token.approve(address(sablier), type(uint256).max);
vm.startPrank(admin);
sablier.setProtocolFee(IERC20(address(token)), ud60x18(0.1e18));
vm.stopPrank();
}
function testStuckFundsAnalysis() public {
uint256 streamId = _createStreamWithOddAmount();
uint256 initialBalance = token.balanceOf(address(sablier));
vm.warp(block.timestamp + 50);
uint256 totalWithdrawn = 0;
uint256 totalProtocolFees = 0;
vm.startPrank(bob);
for (uint256 i = 0; i < 5; i++) {
uint128 withdrawable = sablier.withdrawableAmountOf(streamId);
if (withdrawable > 0) {
(uint128 withdrawn, uint128 feeAmount) = sablier.withdraw(streamId, bob, withdrawable);
totalWithdrawn += withdrawn;
totalProtocolFees += feeAmount;
console2.log("Withdrawal", i + 1);
console2.log(" Withdrawable Amount:", withdrawable);
console2.log(" Withdrawn Amount:", withdrawn);
console2.log(" Protocol Fee:", feeAmount);
}
vm.warp(block.timestamp + 1);
}
vm.stopPrank();
uint256 finalBalance = token.balanceOf(address(sablier));
uint256 protocolRevenue = sablier.protocolRevenue(IERC20(address(token)));
console2.log("\nFinal Analysis:");
console2.log("Initial Balance:", initialBalance);
console2.log("Total Withdrawn:", totalWithdrawn);
console2.log("Total Protocol Fees:", totalProtocolFees);
console2.log("Final Contract Balance:", finalBalance);
console2.log("Protocol Revenue:", protocolRevenue);
console2.log("Stuck Amount:", finalBalance - protocolRevenue);
assertEq(
initialBalance,
totalWithdrawn + totalProtocolFees + finalBalance - protocolRevenue,
"Balance accounting mismatch"
);
uint256 stuckFunds = finalBalance - protocolRevenue;
assertTrue(stuckFunds > 0, "Should have stuck funds");
console2.log("Stuck funds:", stuckFunds);
assertEq(protocolRevenue, totalProtocolFees, "Protocol fee mismatch");
assertEq(finalBalance, stuckFunds + protocolRevenue, "Final balance breakdown mismatch");
}
function _createStreamWithOddAmount() internal returns (uint256) {
uint256 amount = 100e18 + 123;
token.mint(alice, amount);
vm.startPrank(alice);
uint256 streamId = sablier.create(
alice,
bob,
ud21x18(1e18),
IERC20(token),
true
);
sablier.deposit(streamId, uint128(amount), alice, bob);
vm.stopPrank();
return streamId;
}
}
run with: forge test --match-test testStuckFundsAnalysis -vvvv
Initial Balance: 100.000000000000000123 tokens
After 5 withdrawals:
First withdrawal (50 tokens available):
Withdrawn: 45.0 tokens
Protocol Fee: 5.0 tokens
Subsequent withdrawals (1 token each):
Withdrawn: 0.9 tokens
Protocol Fee: 0.1 tokens
Final State:
Total Withdrawn: 48.6 tokens
Total Protocol Fees: 5.4 tokens
Final Contract Balance: 51.400000000000000123 tokens
Protocol Revenue: 5.4 tokens
Stuck Amount: 46.000000000000000123 tokens
The issue appears to be that:
A significant amount (46+ tokens) is stuck in the contract
This amount cannot be withdrawn through normal means
The stuck funds are not part of protocol fees
Root Causes:
Rate Calculation: The stream rate (1 token per second) combined with time-based calculations may lead to rounding issues
Protocol Fee Implementation: The 10% fee calculation might cause small amounts to be unwithdrawable
Withdrawal Logic: The withdrawal mechanism doesn't handle the entire available balance
Test Results:
Initial Balance: 100.000000000000000123
Total Withdrawn: 48.600000000000000000
Total Protocol Fees: 5.400000000000000000
Final Contract Balance: 51.400000000000000123
Protocol Revenue: 5.400000000000000000
Stuck Amount: 46.000000000000000123
This demonstrates that 46% of the initial deposit becomes permanently locked in the contract.
Tools Used
Foundry Testing Framework
Manual Code Review
Custom Test Suite for Fund Flow Analysis
Recommendations
Implement Emergency Withdrawal Function:
function emergencyWithdraw(uint256 streamId) external {
require(_streams[streamId].sender == msg.sender, "Not sender");
uint256 stuckAmount = _streams[streamId].balance;
_streams[streamId].balance = 0;
_streams[streamId].token.safeTransfer(msg.sender, stuckAmount);
emit EmergencyWithdrawal(streamId, msg.sender, stuckAmount);
}
Improve Withdrawal Logic:
function _withdraw(uint256 streamId, address to, uint128 amount) internal {
uint128 remainingAfterWithdraw = _streams[streamId].balance - amount;
if (remainingAfterWithdraw > 0 && remainingAfterWithdraw < DUST_THRESHOLD) {
amount = _streams[streamId].balance;
}
}
Add Protocol Fee Recovery:
function sweepProtocolFees(IERC20 token) external onlyAdmin {
uint256 fees = protocolRevenue[token];
require(fees > 0, "No fees to sweep");
protocolRevenue[token] = 0;
token.safeTransfer(msg.sender, fees);
emit ProtocolFeesSwept(token, fees);
}
