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

`tokenURI()` Does Not Follow EIP-721

albahaca

Summary

The tokenURI() function in the SablierV2NFTDescriptor contract does not adhere to the EIP-721 standard, which mandates that the function should throw an error if _tokenId is not a valid NFT. This can lead to inconsistencies and unexpected behavior when interacting with non-minted NFTs.

Vulnerability Details

According to the EIP-721 standard, the tokenURI() function must throw an error if the _tokenId provided is not valid. However, the current implementation in SablierV2NFTDescriptor.sol does not enforce this requirement. If the NFT corresponding to the streamId has not been minted, tokenURI() should revert but currently does not.

File: v2-core/src/SablierV2NFTDescriptor.sol
47 function tokenURI(IERC721Metadata sablier, uint256 streamId) external view override returns (string memory uri) {
48 TokenURIVars memory vars;
49
50 // Load the contracts.
51 vars.sablier = ISablierV2Lockup(address(sablier));
52 vars.sablierModel = mapSymbol(sablier);
53 vars.sablierStringified = address(sablier).toHexString();
54 vars.asset = address(vars.sablier.getAsset(streamId));
55 vars.assetSymbol = safeAssetSymbol(vars.asset);
56 vars.depositedAmount = vars.sablier.getDepositedAmount(streamId);
57
58 // Load the stream's data.
59 vars.status = stringifyStatus(vars.sablier.statusOf(streamId));
60 vars.streamedPercentage = calculateStreamedPercentage({
61 streamedAmount: vars.sablier.streamedAmountOf(streamId),
62 depositedAmount: vars.depositedAmount
63 });
64
65 // Generate the SVG.
66 vars.svg = NFTSVG.generateSVG(
67 NFTSVG.SVGParams({
68 accentColor: generateAccentColor(address(sablier), streamId),
69 amount: abbreviateAmount({ amount: vars.depositedAmount, decimals: safeAssetDecimals(vars.asset) }),
70 assetAddress: vars.asset.toHexString(),
71 assetSymbol: vars.assetSymbol,
72 duration: calculateDurationInDays({
73 startTime: vars.sablier.getStartTime(streamId),
74 endTime: vars.sablier.getEndTime(streamId)
75 }),
76 sablierAddress: vars.sablierStringified,
77 progress: stringifyPercentage(vars.streamedPercentage),
78 progressNumerical: vars.streamedPercentage,
79 status: vars.status,
80 sablierModel: vars.sablierModel
81 })
82 );
83
84 // Performs a low-level call to handle older deployments that miss the `isTransferable` function.
85 (vars.success, vars.returnData) =
86 address(vars.sablier).staticcall(abi.encodeCall(ISablierV2Lockup.isTransferable, (streamId)));
87
88 // When the call has failed, the stream NFT is assumed to be transferable.
89 vars.isTransferable = vars.success ? abi.decode(vars.returnData, (bool)) : true;
90
91 // Generate the JSON metadata.
92 vars.json = string.concat(
93 '{"attributes":',
94 generateAttributes({
95 assetSymbol: vars.assetSymbol,
96 sender: vars.sablier.getSender(streamId).toHexString(),
97 status: vars.status
98 }),
99 ',"description":"',
100 generateDescription({
101 sablierModel: vars.sablierModel,
102 assetSymbol: vars.assetSymbol,
103 sablierStringified: vars.sablierStringified,
104 assetAddress: vars.asset.toHexString(),
105 streamId: streamId.toString(),
106 isTransferable: vars.isTransferable
107 }),
108 '","external_url":"https://sablier.com","name":"',
109 generateName({ sablierModel: vars.sablierModel, streamId: streamId.toString() }),
110 '","image":"data:image/svg+xml;base64,',
111 Base64.encode(bytes(vars.svg)),
112 '"}'
113 );
114
115 // Encode the JSON metadata in Base64.
116 uri = string.concat("data:application/json;base64,", Base64.encode(bytes(vars.json)));
117 }

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2NFTDescriptor.sol#L47-L117

File: v2-core/src/abstracts/SablierV2Lockup.sol
209 function tokenURI(uint256 streamId) public view override(IERC721Metadata, ERC721) returns (string memory uri) {
210 // Check: the stream NFT exists.
211 _requireOwned({ tokenId: streamId });
212
213 // Generate the URI describing the stream NFT.
214 uri = nftDescriptor.tokenURI({ sablier: this, streamId: streamId });
215 }

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L209-L215

Impact

Failure to revert when the tokenId is invalid can lead to unexpected behavior and inconsistencies. It might cause issues in applications relying on the standard behavior of tokenURI(), potentially affecting the functionality of NFTs in user interfaces and other integrations.

Tools Used

  • Manual code review

  • EIP-721 documentation

Recommendations

  • Add a Validity Check: Ensure that the tokenURI() function includes a check to verify that the streamId corresponds to a valid, minted NFT. If the streamId is not valid, the function should revert with an appropriate error message.

Example:

require(_exists(streamId), "ERC721Metadata: URI query for nonexistent token");
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.