code-423n4 / 2022-05-opensea-seaport-findings

1 stars 0 forks source link

Proof for NFT with token id = 0 won't be checked #120

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

The proof (criteria) for NFTs with a zero token id won't be checked, because of a zero token id represents an item with no criteria.

Proof of Concept

This if check skips the check for the proof of NFT with token id = 0.

if (identifierOrCriteria != uint256(0)) {
    // Verify identifier inclusion in criteria root using proof.
    _verifyProof(
        criteriaResolver.identifier,
        identifierOrCriteria,
        criteriaResolver.criteriaProof
    );
}

Tools Used

Remix, VS Code and my brain.

Recommended Mitigation Steps

Mark item with no criteria in other way than zero token id, for example a boolean value or anything else.

0age commented 2 years ago

if a specific tokenId (in this case, 0) is supplied, then it is not a criteria-based item and so does not need to supply a proof at all.