DatingDapp

Summary

ThetokenURI(uint256 tokenId) function in the SoulboundProfileNFT contract uses abi.encodePacked() with multiple dynamic types (strings) to construct JSON metadata. This practice can lead to hash collisions when used with keccak256() or similar functions. The correct approach is to use abi.encode() instead, which ensures proper 32-byte padding and prevents concatenation conflicts.

Vulnerability Details

Code Snippet:

Why is this an issue?

1. abi.encodePacked() concatenates variables without enforcing 32-byte padding.

2. Dynamic types (like string and bytes) can merge incorrectly, leading to different inputs producing the same hash.

3. This hash collision can cause unexpected behavior in contracts that rely on keccak256() for unique identifiers.

PoC

Consider two different sets of inputs that could produce the same hash:

A malicious actor could exploit this to bypass validation checks or manipulate metadata references.

Impact

High Severity: Hash collisions can lead to unexpected behavior in metadata retrieval, potentially causing incorrect NFT attributes to be displayed.

Metadata Integrity Risk: If a function relies on keccak256(abi.encodePacked(...)) to generate a unique ID, two different sets of inputs may yield the same result, compromising NFT uniqueness.

Potential Exploits: Attackers could manipulate JSON metadata to misrepresent attributes, affecting the value and trust in the NFT collection.

Tools Used

Manual Review (to confirm hash collision risk)

Aderyn

Recommendations

To prevent hash collisions, replace abi.encodePacked() with abi.encode() or use explicit concatenation methods (bytes.concat()).