Submitted by cmichel, also found by frangio and Spearbit
CriteriaResolution.sol#L157
The protocol allows specifying several tokenIds to accept for a single offer.
A merkle tree is created out of these tokenIds and the root is stored as the identifierOrCriteria for the item.
The fulfiller then submits the actual tokenId and a proof that this tokenId is part of the merkle tree.
There are no real verifications on the merkle proof that the supplied tokenId is indeed a leaf of the merkle tree.
Its possible to submit an intermediate hash of the merkle tree as the tokenId and trade this NFT instead of one of the requested ones.
This leads to losses for the offerer as they receive a tokenId that they did not specify in the criteria.
Usually, this criteria functionality is used to specify tokenIds with certain traits that are highly valuable. The offerer receives a low-value token that does not have these traits.
Alice wants to buy either NFT with tokenId 1 or tokenId 2.
She creates a merkle tree of it and the root is hash(1||2) = 0xe90b7bceb6e7df5418fb78d8ee546e97c83a08bbccc01a0644d599ccd2a7c2e0.
She creates an offer for this criteria.
An attacker can now acquire the NFT with tokenId 0xe90b7bceb6e7df5418fb78d8ee546e97c83a08bbccc01a0644d599ccd2a7c2e0 (or, generally, any other intermediate hash value) and fulfill the trade.
Heres a forge test (gist) that shows the issue for the situation mentioned in Example.
Usually, this is fixed by using a type-byte that indicates if one is computing the hash for a leaf or not.
An elegant fix here is to simply use hashes of the tokenIds as the leaves - instead of the tokenIds themselves. (Note that this is the natural way to compute merkle trees if the data size is not already the hash size.)
Then compute the leaf hash in the contract from the provided tokenId:
There cant be a collision between a leaf hash and an intermediate hash anymore as the former is the result of hashing 32 bytes, while the latter are the results of hashing 64 bytes.
Note that this requires off-chain changes to how the merkle tree is generated. (Leaves must be hashed first.)
0age (OpenSea) confirmed, but disagreed with severity
HardlyDifficult (judge) decreased severity to Medium
0xleastwood (judge) commented:
0age (OpenSea) resolved:
