Weather Witness

First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: low
Likelihood: medium
Invalid

Metadata Collision via abi.encodePacked()

Improper use of abi.encodePacked() in metadata construction introduces potential hash collision risks that could lead to NFT identity forgery or off-chain verification failures.

Description

  • In ERC721 contracts, the tokenURI() function generates metadata for an NFT that is used to render name, image, owner, and other traits. This data is typically constructed as a JSON string and base64 encoded.

  • In the WeatherNft::tokenURI() function, abi.encodePacked() is used to concatenate strings when constructing the metadata. While this may seem efficient, it can lead to hash collisions when used in combination with keccak256() or other hash-based comparisons. This is a known risk due to the ambiguous, non-delimited encoding produced by abi.encodePacked.

function tokenURI(uint256 tokenId) public view override returns (string memory) {
_requireOwned(tokenId);
string memory image = s_weatherToTokenURI[s_tokenIdToWeather[tokenId]];
//@audit use abi.encode() instead of abi.encodePacked() to prevent hash collisions
@> bytes memory jsonData = abi.encodePacked(
@> '{"name": "Weathear NFT", "user": "',
@> Strings.toHexString(_ownerOf(tokenId)),
@> '", "image": "',
@> image, '"}'
);
string memory base64TransformedData = Base64.encode(jsonData);
return string.concat(_baseURI(), base64TransformedData);
}

Risk

Likelihood:

  • Developers frequently use abi.encodePacked() for string concatenation, and it may later be reused in ways involving hashing (e.g., caching metadata hashes, verifying signatures).

  • No delimiter or length information is preserved between arguments, so multiple different inputs can yield the same packed byte string.


Impact:

  • An attacker could craft multiple distinct inputs (e.g., addresses or image URIs) that collide to the same hash, which may break future assumptions about uniqueness.

  • If tokenURI() output is ever hashed for off-chain verification, NFTs with different owners or metadata could produce the same result, enabling forgery or misrepresentation.


Proof of Concept

Created a simple Foundry test showing that different inputs produce the same abi.encodePacked() result, which would produce the same base64 metadata hash if used inside tokenURI():

function testAbiEncodePackedCollision() public {
bytes memory packed1 = abi.encodePacked("abc", "def");
bytes memory packed2 = abi.encodePacked("abcd", "ef");
assertEq(keccak256(packed1), keccak256(packed2), "Hash collision detected");
console.log("Packed1 hash: ");
console.logBytes32(keccak256(packed1)); //0xacd0c377fe36d5b209125185bc3ac41155ed1bf7103ef9f0c2aff4320460b6df
console.log("Packed2 hash: ");
console.logBytes32(keccak256(packed2)); //0xacd0c377fe36d5b209125185bc3ac41155ed1bf7103ef9f0c2aff4320460b6df
}

Test Output

Ran 1 test for test/WeatherNftForkTest.t.sol:WeatherNftForkTest
[PASS] testAbiEncodePackedCollision() (gas: 13235)
Logs:
Packed1 hash:
0xacd0c377fe36d5b209125185bc3ac41155ed1bf7103ef9f0c2aff4320460b6df
Packed2 hash:
0xacd0c377fe36d5b209125185bc3ac41155ed1bf7103ef9f0c2aff4320460b6df
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.15ms (1.42ms CPU time)
Ran 1 test suite in 160.79ms (8.15ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

abi.encode(...) is safer and unambiguous:

  • It adds length prefixes and type separation.

  • Prevents subtle collisions if tokenURI() output is ever hashed, compared, or verified off-chain.

function tokenURI(
uint256 tokenId
) public view override returns (string memory) {
_requireOwned(tokenId);
- bytes memory jsonData = abi.encodePacked(
- '{"name": "Weathear NFT", "user": "',
- Strings.toHexString(_ownerOf(tokenId)),
- '", "image": "',
- image, '"}'
- );
+ bytes memory jsonData = abi.encode(
+ '{"name": "Weathear NFT", "user": "',
+ Strings.toHexString(_ownerOf(tokenId)),
+ '", "image": "',
+ image, '"}'
+ );
string memory base64TransformedData = Base64.encode(jsonData);
return string.concat(_baseURI(), base64TransformedData);
}
Updates

Appeal created

bube Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[Invalid] Hash collision

This is informational. The `user` and the `image` are always different for each `tokenURI`.

Support

FAQs

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