Weather Witness
First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Usage of `abi.encodePacked()` causes a Hash Collision

0xshield

Root + Impact

Description

The tokenURI() function is expected to generate unique and collision-free metadata URIs for each NFT. However, the use of abi.encodePacked() when building jsonData can lead to hash collisions or unexpected concatenation results due to Solidity's packed encoding behavior with dynamic types.

This can result in incorrect token metadata being returned for NFTs, especially as the number of tokens grows.

// Root cause in the codebase with @> marks to highlight the relevant section
270: bytes memory jsonData = @>abi.encodePacked<@(
'{"name":"WeatherNFT #',
Strings.toString(tokenId),
'", "description":"NFT representing weather in ',
info.pincode,
', ',
info.isoCode,
'", "weather":"',
weatherString,
'", "timestamp":"',
Strings.toString(info.lastFulfilledAt),
'"}'
);

Risk

Likelihood:

  • This can occur when the protocol generates many NFTs and the concatenated fields overlap in byte structure due to dynamic type packing.

  • Since dynamic fields like pincode, isoCode, and weatherString are used, this issue could surface as the dataset grows.

Impact:

  • Two NFTs may produce the same tokenURI, showing incorrect metadata.

  • A user might view metadata belonging to another user, breaking the uniqueness guarantee of NFTs.

Proof of Concept

Add the following code on the end of WeatherNftForkTest.t.sol::test_weatherNFT_Workflow():

Code
uint256 count = 5; // number of NFTs
string[] memory tokenURIs = new string[](count);
​
uint256 nextTokenId = weatherNft.s_tokenCounter();
​
for (uint256 i = 0; i < count; i++) {
uint256 currTokenId = nextTokenId + i;
​
// Mint NFT
vm.startPrank(user);
linkToken.approve(address(weatherNft), initLinkDeposit);
vm.recordLogs();
weatherNft.requestMintWeatherNFT{value: weatherNft.s_currentMintPrice()}(
pincode,
isoCode,
registerKeeper,
heartbeat,
initLinkDeposit
);
vm.stopPrank();
​
// Retrieve reqId
Vm.Log[] memory logsMint = vm.getRecordedLogs();
bytes32 mintReqId;
for (uint256 j = 0; j < logsMint.length; j++) {
if (logsMint[j].topics[0] == keccak256("WeatherNFTMintRequestSent(address,string,string,bytes32)")) {
(, , , mintReqId) = abi.decode(logsMint[j].data, (address, string, string, bytes32));
break;
}
}
​
// Fulfill & mint
vm.prank(functionsRouter);
weatherNft.handleOracleFulfillment(mintReqId, abi.encode(WeatherNftStore.Weather.SUNNY), "");
​
vm.prank(user);
weatherNft.fulfillMintRequest(mintReqId);
​
string memory uri = weatherNft.tokenURI(currTokenId);
tokenURIs[i] = uri;
​
// uniqueness check
for (uint256 k = 0; k < i; k++) {
assertNotEq(tokenURIs[k], uri, "Duplicate tokenURI detected");
}
}
This assertion fails due to `abi.encodePacked()` causing a collision.

Recommended Mitigation

- bytes memory jsonData = abi.encodePacked(
+ bytes memory jsonData = abi.encode(

Alternatively, use a proper JSON struct and a string library if you need to manually construct JSON.

Updates

Appeal created

bube Lead Judge 4 months 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.