`_verifyProof` allows empty proofs (allows malleable transactions)

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/CriteriaResolution.sol#L155 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/CriteriaResolution.sol#L241

Vulnerability details

Impact

_verifyProof allows empty proofs and in that case it expects the leaf to equal the root, because no hashing and iteration is taking place. The purpose of the tree is to hold multiple accepted tokenIds, where the consideration contains one and proving its existence in the order via the proof. Because of this the leaf equals to a single tokenId.

This can manifest in distinct issues:

1. A criteria order with an empty proof is the same as a non-criteria order with the exception of the ItemType is differing (e.g. ItemType.ERC721 vs ItemType.ERC721_WITH_CRITERIA). This is a slight malleability symptom.

2. In the case of leaf=0 (tokenId=0) the _verifyProof function is not even executed.

Proof of Concept

Context: CriteriaResolution.sol#L241, CriteriaResolution.sol#L155

Tools Used

Manual review

Recommended Mitigation Steps

Hash the leaf node or disallow empty proofs.

[0age]

This is valid, and we intend to fix — it is also a subset of the more generalized finding on criteria proofs where intermediate nodes of the proof can be used as leaves (and has the same mitigation, hashing leaf nodes)

[HardlyDifficult]

Dupe of https://github.com/code-423n4/2022-05-opensea-seaport-findings/issues/168