Sablier
Sablier
DeFiFoundry
53,440 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Streams can be created with ERC721s as the underlying payment asset

palmcivet

Description When a sender creates a payment stream, they are intended to specify the address of an ERC20 token, which the streamed payment will be in. The README states that the Sablier Protocol is not compatible with any token standard other than ERC20. However it is possible for a sender to create a payment stream by specifying an ERC721 as the underlying payment asset, if they also pass the ERC721's tokenID as the params.totalAmount.

Impact Senders who create streams with ERC721s as the underlying payment asset will lose access to their ERC721 token and cause confusion to recipients who observed events such as CreateLockupLinearStream being emitted.

Proof of Concept

Code

Place the following code in a Foundry Forge test file.

import { Events } from "../utils/Events.sol";
.
.
.
function test_stream_nft() public {
vm.startPrank(sender);
TestNft nft = new TestNft();
address nftAddress = address(nft);
​
// Define the parameters for createWithTimestamps
params = LockupLinear.CreateWithTimestamps({
sender: sender,
recipient: recipient,
totalAmount: uint128(1),
asset: IERC20(nftAddress),
cancelable: true,
transferable: true,
timestamps: LockupLinear.Timestamps({
start: uint40(block.timestamp), // Stream starts now
cliff: uint40(block.timestamp + 60), // Cliff period ends in 60 seconds
end: uint40(block.timestamp + 3600) // Stream ends in 1 hour
}),
broker: Broker({ account: address(0), fee: wrap(0) })
});
​
// create stream and expect emit
nft.approve(address(sablier), 1);
vm.expectEmit({ emitter: address(sablier) });
emit CreateLockupLinearStream(
1,
sender,
sender,
recipient,
Lockup.CreateAmounts({ deposit: 1, brokerFee: 0 }),
IERC20(nftAddress),
true,
true,
LockupLinear.Timestamps({
start: uint40(block.timestamp),
cliff: uint40(block.timestamp + 60),
end: uint40(block.timestamp + 3600)
}),
address(0)
);
sablier.createWithTimestamps(params);
vm.stopPrank();
​
// assert sablier is now the owner of the nft
assertEq(nft.ownerOf(1), address(sablier));
​
// Set block timestamp to the end time of the stream
vm.warp(params.timestamps.end);
​
// recipient cannot withdraw the "payment" nft locked in sablier
vm.prank(recipient);
vm.expectRevert();
sablier.withdraw(1, recipient, uint128(1));
}
.
.
.
import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
​
contract TestNft is ERC721 {
address private immutable owner;
​
constructor() ERC721("Test NFT", "TNFT") {
owner = msg.sender;
_mint(msg.sender, 1);
}
}

Recommended Mitigation Consider checking the params.asset has decimals() (or something else unique to ERC20s that ERC721s do not have).

+ function isERC20(address token) internal view returns (bool) {
+ (bool success, bytes memory data) = token.staticcall(
+ abi.encodeWithSignature("decimals()")
+ );
+ return success && data.length > 0;
+ }
​
+ modifier onlyERC20(address token) {
+ require(isERC20(token), "Token is not an ERC20 token");
+ _;
+ }
​
function createWithTimestamps(LockupDynamic.CreateWithTimestamps calldata params)
external
override
noDelegateCall
+ onlyERC20(address(params.asset))
returns (uint256 streamId)
{
// existing code
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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