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

[M-1] Missing critical stream information in NFT metadata could lead to misleading sales, particularly a problem with streams that are in a non-flowing state and remain transferable.

casinocompiler

[M-1] Missing critical stream information in NFT metadata could lead to misleading sales, particularly a problem with streams that are in a non-flowing state and remain transferable.

Description:

The Sablier Flow protocol mints NFTs representing streams which can be made transferable. Currently, although the NFT has not been implemented yet, IFlowNFTDescriptor.sol::tokenURI() suggests only the streamId and the sablierFlow contract address will be used in the metadata; critical underlying information such as stream status, rate or balance will not be updated. Additionally, the NFT remains transferrable in non-flowing states. This could lead to users purchasing NFTs representing depleted, void or paused streams on NFT marketplaces without understanding the true state of the stream.

Impact:

  • NFT marketplaces typically display only NFT metadata, making due diligence harder:

    • NFT buyers may purchase stream NFTs without understanding the actual value/status of the underlying stream.

    • Financial loss possible if NFTs are sold at prices not reflecting actual stream status

  • Trust issues for the protocol if stream NFTs are misused:

    • It would become social norm to avoid interacting with Flow NFTs, thus rendering the whole NFT functionality useless.

Proof of Concept:

This PoC will be using a voided stream example as this scenario is the most impactful one and can be easily manipulated.

  1. Bad actor creates a new stream through a stream providing service.

  2. Bad actor receives NFT for stream.

  3. Bad actor has voided the stream intentionally immediately.

  4. Bad actor sells NFT to buyer who was unaware of void function for streams.

As the access controls for voiding a stream can be done by the Sender/Recipient/Administrator, multiple different combinations of manipulation attacks could occur e.g the bad actor can create a stream themselves(sender) to themselves(recipient), void and list NFTs repeatedly.

Proof of Code

Create the following test contract in the repo tests/integration folder:

// SPDX-License-Identifier: UNLICENSED
​
pragma solidity >=0.8.22;
​
import { Integration_Test } from "../Integration.t.sol"; // import PATH may be different
import { UD21x18, ud21x18 } from "@prb/math/src/UD21x18.sol";
import { Flow } from "src/types/DataTypes.sol";
import { IERC721 } from "@openzeppelin/contracts/interfaces/IERC721.sol";
​
contract NFTTest is Integration_Test {
address streamProviderService = makeAddr("streamProviderService");
address badActor = makeAddr("badActor");
address userOne = makeAddr("userOne");
function setUp() public override {
// Call parent setUp which sets up admin, users, tokens etc
super.setUp();
}
​
function testMisleadingNFTSale() public {
UD21x18 initialRate = ud21x18(uint128(RATE_PER_SECOND.unwrap()));
uint256 block_i = block.number;
​
// streamProviderService creates transferable stream to badActor in block_i
vm.warp(block_i);
vm.startPrank(streamProviderService);
uint256 streamId = flow.create({
sender: streamProviderService,
recipient: badActor,
ratePerSecond: initialRate,
token: dai,
transferable: TRANSFERABLE
});
vm.stopPrank();
​
// badActor voids the stream in the next block
vm.warp(block_i + 1);
vm.startPrank(badActor);
flow.void(streamId);
vm.stopPrank();
​
// Assert stream is voided
// NOTE :: tokenId is 2 as this test inherits Integration_Test which creates previous streams.
// NOTE :: 4 == flow.status.VOIDED
assertEq(uint256(flow.statusOf(2)), 4);
​
// Assert NFT still exists and is owned by badActor
assertEq(flow.ownerOf(streamId), badActor);
​
// Simulate someone buying voided NFT :: transfer NFT to userOne
// After many days (10 days = 71680 blocks)
vm.warp(block_i + 71680);
vm.startPrank(badActor);
IERC721(flow).transferFrom(badActor, userOne, 2);
vm.stopPrank();
​
// Assert userOne owns voided stream NFT
assertEq(flow.ownerOf(streamId), userOne);
}
}

Recommended Mitigation:

  1. Include critical stream information in dynamic NFT metadata format.

  2. Consider making NFTs non-transferable when a stream is paused / depleted / void.

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.