Root + Impact

Description

• The FestivalPass contract uses COLLECTION_ID_SHIFT = 128 in the encodeTokenId and decodeTokenId functions to generate and parse token IDs for memorabilia NFTs in the redeemMemorabilia function. This constant is incorrectly set to 128 (representing a 7-bit shift) instead of 1 << 128 (a 128-bit shift), leading to token ID collisions when itemId >= 128. As a result, different (collectionId, itemId) pairs can produce identical tokenId values, breaking the uniqueness of ERC1155 NFTs and causing incorrect metadata retrieval in getMemorabiliaDetails.

Risk

• COLLECTION_ID_SHIFT = 128 shifts collectionId left by 7 bits, placing it in bits 7–15 of the uint256 token ID. When itemId exceeds 127, it overlaps with these bits, causing collisions. For example:

• (collectionId = 100, itemId = 128): tokenId = (100 << 128) + 128 = 12928.

• (collectionId = 101, itemId = 0): tokenId = (101 << 128) + 0 = 12928.

Both produce the same tokenId, violating ERC1155 uniqueness.

• Decoding Error: decodeTokenId uses itemId = uint256(uint128(tokenId)), which incorrectly extracts 128 bits instead of the lower 7 bits, returning invalid itemId values (e.g., 12928 instead of 128).

• Context: In redeemMemorabilia, collectionId starts at 100 (nextCollectionId), and itemId increments from 1 up to maxSupply (e.g., 1000). Since itemId can exceed 127, collisions are likely.

Impact:

*NFT Uniqueness: Collisions cause multiple NFTs (from different collections or editions) to share the same tokenId, leading to incorrect balanceOf counts and ownership disputes.

*Metadata Errors: getMemorabiliaDetails returns incorrect collectionId and itemId, displaying wrong collection names or edition numbers.

*Economic Risk: If memorabilia NFTs are valuable (e.g., rare collectibles), collisions undermine the festival’s NFT ecosystem, potentially reducing user trust or causing financial disputes.

Likelihood: High, as maxSupply can be large (e.g., 1000), and itemId quickly exceeds 127.

Recommended Mitigation

• Update COLLECTION_ID_SHIFT to 1 << 128 to shift collectionId by 128 bits, ensuring no overlap with itemId. Also, fix decodeTokenId to correctly extract itemId: