Root + Impact

Insufficient bit-spacing in encodeTokenId() allows hash collision

Description

The encodeTokenId() function is responsible for generating unique uint256 token IDs from two or more input parameters. The current implementation packs these parameters using simple arithmetic (e.g., addition or multiplication) rather than a collision-resistant encoding scheme such as bitwise shifting with reserved bit ranges or abi.encode followed by keccak256.

As a result, different combinations of input parameters can produce identical output values. For example, if the function computes tokenId = a * 1000 + b, then the inputs (1, 100) and (2, 100) may collide with inputs like (0, 1100) if parameter ranges are not strictly bounded. The lack of uniqueness guarantees means two semantically distinct tokens — representing different items, tiers, or memberships — can end up sharing the same uint256 ID.

In Solidity's ERC-721 standard, token IDs are the primary key for ownership. If two distinct tokens share an ID, only one can exist in the ownership mapping at a time. The second mint will either overwrite the first owner's record or revert, depending on the implementation. This breaks the fundamental NFT guarantee of uniqueness and can be weaponized by an attacker who deliberately crafts input parameters that collide with a high-value token ID already owned by another user, effectively stealing or overwriting ownership.

Risk

Likelihood:

This vulnerability is rated Medium likelihood. The collision requires an attacker to understand the encoding function's arithmetic and identify a colliding parameter pair. This is trivially achievable by anyone who can read the contract's bytecode or source code. The difficulty is not technical — it is a matter of motivation. In a competitive or adversarial environment (e.g., a popular NFT launch), the likelihood of someone probing for collisions is significant.

Impact:

This vulnerability is rated Critical impact. A successful collision directly compromises token ownership — the most fundamental property of an NFT contract. An attacker who identifies a colliding parameter pair can mint a token that overwrites an existing owner's record, or prevent a legitimate token from being minted at all. Either outcome constitutes irreversible asset theft or destruction. Since token IDs are immutable once assigned, there is no in-contract remediation path without a full migration.

Proof of Concept

By choosing parameters where the arithmetic encoding produces the same result, the attacker can predict and target an already-minted token ID. If _safeMint is used, the second call reverts (since the token already exists), but this still constitutes a griefing attack — the attacker can permanently block any token whose ID they can predict. If a custom mint function is used without the duplicate check, ownership is silently overwritten.

Recommended Mitigation

The recommended fix is to use bitwise packing with clearly defined, non-overlapping bit ranges. Shifting typeId left by 128 bits and OR-ing with index guarantees uniqueness as long as each parameter stays within its 128-bit range. For maximum safety, use keccak256(abi.encode(typeId, index)) — abi.encode produces a collision-free byte representation, and hashing it gives a uniformly distributed unique identifier. Always add require bounds checks on inputs to ensure parameters cannot exceed their allocated bit range.