GivingThanks

Summary

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). Unless there is a compelling reason, abi.encode should be preferred.

Vulnerability Details

In Solidity, abi.encodePacked concatenates data into a single byte stream without padding, which may produce unexpected results when dynamic types like strings are concatenated. This is especially risky in contexts where abi.encodePacked output is hashed, as different data inputs could produce identical hash results due to compact encoding. Although this function does not perform hashing, it is best practice to use abi.encode or bytes.concat() when concatenating dynamic types for consistency and to avoid future code misinterpretation.

• Root Cause: abi.encodePacked is used with dynamic string data types within the _createTokenURI function.

• Instances: This issue occurs in the concatenation of the JSON metadata for token URIs.

Impact

Using abi.encodePacked with dynamic types can lead to hash collisions if reused in hashing contexts, as different inputs may produce identical outputs. This practice also creates a misleading coding pattern, potentially leading to data integrity issues in future contract changes or expansions.

Tools Used

• Manual Code Review

• Aderyn

• Foundry

Recommendations

Replace abi.encodePacked with abi.encode: Modify the function to use abi.encode to ensure consistency with best practices and avoid potential data collisions in any future use cases.