Summary
There are no checks done during a stream's creation to make sure that the sender address is not the same as the recipient address. This is intended as if the stream's transferable is set to true, this stream(NFT) can later be transferred to reflect a debt payment. However, there are no checks done to make sure the sender address is not the same as the recipient address when the stream is not transferable, making such streams useless.
Vulnerability Details
The sender can create a non transferable stream with the create or createAndDeposit functions via setting the transferable input to false.
function create(
address sender,
address recipient,
UD21x18 ratePerSecond,
IERC20 token,
bool transferable
)
external
override
noDelegateCall
returns (uint256 streamId)
{
streamId = _create(sender, recipient, ratePerSecond, token, transferable);
}
function createAndDeposit(
address sender,
address recipient,
UD21x18 ratePerSecond,
IERC20 token,
bool transferable,
uint128 amount
)
external
override
noDelegateCall
returns (uint256 streamId)
{
streamId = _create(sender, recipient, ratePerSecond, token, transferable);
_deposit(streamId, amount);
}
function _create(
address sender,
address recipient,
UD21x18 ratePerSecond,
IERC20 token,
bool transferable
)
internal
returns (uint256 streamId)
{
if (sender == address(0)) {
revert Errors.SablierFlow_SenderZeroAddress();
}
uint8 tokenDecimals = IERC20Metadata(address(token)).decimals();
if (tokenDecimals > 18) {
revert Errors.SablierFlow_InvalidTokenDecimals(address(token));
}
streamId = nextStreamId;
_streams[streamId] = Flow.Stream({
balance: 0,
isStream: true,
isTransferable: transferable,
isVoided: false,
ratePerSecond: ratePerSecond,
sender: sender,
snapshotDebtScaled: 0,
snapshotTime: uint40(block.timestamp),
token: token,
tokenDecimals: tokenDecimals
});
unchecked {
nextStreamId = streamId + 1;
}
_mint({ to: recipient, tokenId: streamId });
emit ISablierFlow.CreateFlowStream({
streamId: streamId,
sender: sender,
recipient: recipient,
ratePerSecond: ratePerSecond,
token: token,
transferable: transferable
});
}
As observed in these functions, there are no checks done to make sure that the sender address is the same as the recipient address when transferable is set to false. Lack of checks allows creating of a stream where the sender can be the same address even if transferable is set to false making this stream effectively useless.
Proof of Concept
Add the following test contract in the tests file and run it to observe the vulnerability. In order to run this test, add a mint function in the MockERC20.sol
pragma solidity >=0.8.22;
import { Test } from "forge-std/src/Test.sol";
import { console } from "forge-std/src/console.sol";
import "../src/SablierFlow.sol";
import "../src/interfaces/IFlowNFTDescriptor.sol";
import "./mocks/ERC20Mock.sol";
contract MockNFTDescriptor is IFlowNFTDescriptor {
function tokenURI(IERC721Metadata, uint256) external pure override returns (string memory) {
return "test_uri";
}
}
contract CustomTest is Test {
SablierFlow private flow;
ERC20Mock private token;
MockNFTDescriptor private nftDescriptor;
address private sender = address(0x1);
address private recipient = address(0x2);
uint128 private amount = 200e8;
uint256 private streamId;
function setUp() public {
token = new ERC20Mock("MockToken", "MTK", 0);
nftDescriptor = new MockNFTDescriptor();
token.mint(sender, amount);
flow = new SablierFlow(address(admin), nftDescriptor);
vm.prank(sender);
token.approve(address(flow), amount);
}
function testCreateStreamSameAddress() public {
uint128 amountToDeposit = 100;
vm.prank(sender);
streamId = flow.createAndDeposit(sender, sender, ud21x18(1e18), token, false, amountToDeposit);
address streamOwner = flow.ownerOf(streamId);
assertEq(streamOwner, address(sender));
bool transferable = flow.isTransferable(streamId);
assertEq(transferable, false);
}
Impact
It is possible to create useless streams with sender address is being the same as the recipient address, that are not transferable.
Impact: Low
Likelihood: Low
Tools Used
Manual review, foundry
Recommendations
Implement checks to make sure that the sender address can not be the same as the recipient address when the stream is non-transferable. An example of this is given below.
function _create(
address sender,
address recipient,
UD21x18 ratePerSecond,
IERC20 token,
bool transferable
)
internal
returns (uint256 streamId)
{
if (!transferable) {
if(sender == recipient){
revert Errors.SablierFlow_SenderIsRecipient();
}
}
