Flow
Sablier
FoundryDeFi
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Voided streams can be transferred which is of no value

sancybars

Discussion

The void function is used to stop a stream. However, if a stream is voided, it can not be restarted meaning the stream is of no value. This is a problem because the stream can still be transferred. This is a concern as voided stream id can of sablier can be transferred, while it is of no consequence to the spender or recipient because either can refund, withdraw respectively. However, the stream id can still be transferred to other users to mislead them about the status and value of the stream.

For Example:
John creates a stream with id 1 and deposits 100 tokens. While the tokens are stream Sarah the recipient had claimed 50 tokens. He then voids the stream. The stream is now of no value to John or Sarah. However, Sarah can still transfer the stream id to Bob to mislead him.

​
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import { Test,console2 } from "forge-std/src/Test.sol";
import "src/SablierFlow.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { ERC20Mock } from "tests/mocks/ERC20Mock.sol";
import "src/abstracts/SablierFlowBase.sol";
import "src/FlowNFTDescriptor.sol";
​
contract SablierFlowWithdrawTest is Test {
UD60x18 internal constant PROTOCOL_FEE = UD60x18.wrap(0.01e18); // 1%
​
SablierFlow sablierFlow;
ERC20Mock testToken;
FlowNFTDescriptor flowDescriptor;
​
address john = address(0x1111);
address sarah = address(0x2222);
address bob = address(0x3333);
​
uint256 streamId;
​
function setUp() public {
// Deploy SablierFlow
sablierFlow = new SablierFlow(address(this), IFlowNFTDescriptor(flowDescriptor));
​
// Deploy a test ERC20 token
testToken = new ERC20Mock("Test Token", "TT", 18);
// Mint initial tokens to Alice
testToken.mint(john, 100e18);
testToken.mint(address(this), 2e18);
testToken.approve(address(sablierFlow), type(uint256).max);
​
// Approve SablierFlow to spend Alice's tokens
vm.startPrank(john);
testToken.approve(address(sablierFlow), type(uint256).max);
​
// Create a stream from Alice to Bob
streamId = sablierFlow.create(
john,
sarah,
UD21x18.wrap(1e18),
testToken,
true
);
vm.stopPrank();
​
streamId = sablierFlow.create(john, sarah, UD21x18.wrap(1e18), testToken, false);
console2.log(streamId);
​
}
​
​
​
function testCanTransferVoidId() public {
//set Protocol fee
vm.prank(address(this));
sablierFlow.setProtocolFee(testToken, PROTOCOL_FEE);
​
//stream balance is zero
uint128 streamBalanceBefore = sablierFlow.getStream(1).balance;
console2.log(streamBalanceBefore);
​
vm.warp(20 seconds);
//sender deposits
vm.prank(john);
sablierFlow.deposit(1, 100e18, john, sarah);
​
//sender voids the stream
vm.startPrank(john);
sablierFlow.void(1);
//claims what is left thats not streamed
sablierFlow.refund(1, 81e18);
vm.stopPrank();
​
vm.startPrank(sarah)// recipient withdraws
sablierFlow.withdraw(1, sarah, 19e18);
//cna still transfer the voided stream
sablierFlow.safeTransferFrom(sarah, bob, 1, "");
sablierFlow.getStream(1);
}
}

During transfer the update logic does not check the status of the streamId

function _update(
address to,
uint256 streamId,
address auth
)
internal
override
updateMetadata(streamId)//@audit can transfer voided streams
returns (address)
{
address from = _ownerOf(streamId);
​
if (from != address(0) && !_streams[streamId].isTransferable) {
revert Errors.SablierFlowBase_NotTransferable(streamId);
}
​
return super._update(to, streamId, auth);
}

Recommendation

Add the notVoided(streamId) modifier in _update.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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