GivingThanks

Summary

_createTokenURI in the GivingThanks contract uses abi.encodePacked() to generate the metadata URI for tokens. Due to the lack of padding with abi.encodePacked, there is a risk of URI collisions, which could undermine the uniqueness of each token URI.

Vulnerability Details

From the solidity documentation:
https://docs.soliditylang.org/en/v0.8.17/abi-spec.html?highlight=collisions#non-standard-packed-mode
> If you use keccak256(abi.encodePacked(a, b)) and both a and b are dynamic types, it is easy to craft collisions in the hash value by moving parts of a into b and vice-versa. More specifically, abi.encodePacked("a", "bc") == abi.encodePacked("ab", "c").

_createTokenURI() is used to generate JSON metadata by concatenating address, date, and amount information with abi.encodePacked(). Because abi.encodePacked() does not add padding between inputs, distinct data could produce identical output under certain conditions, potentially leading to a URI collision.

For example, when encoding different addresses or values of similar structure and length, abi.encodePacked() may not distinguish between them accurately, resulting in identical metadata URIs for different tokens.

This issue exists in _creaTokenURI() as 'abi.encodePacked's inputs are all dynamic:

Impact

• Loss of Token Uniqueness: The ERC721 standard relies on each token having a unique identifier, typically represented by a unique token URI. Collisions could lead to two or more tokens sharing the same URI, compromising token uniqueness and potentially affecting user trust.

• Non-Compliance with ERC721 Standard: ERC721 tokens are expected to have unique identifiers. If metadata URIs collide, the contract may fail to fully comply with the ERC721 standard.

Tools Used

Manual code review

Recommendations

Replace abi.encodePacked() with abi.encode() would ensure each input is distinct, eliminating the risk of unintended concatenation issues.