_transfer#Executor.sol not checking all enum values

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/Executor.sol#L85-L96

Vulnerability details

Impact

Transaction can revert or have unexpected behaviour

Poc

In _transfer#executor.sol you are checking Itemtype values Itemtype.NATIVE, Itemtype.ERC20 and Itemtype.ERC721 after then the last else clause assumes that the Itemtype is a ERC1155.

} else {  @audit assumes Itemtype.ERC1155 without checking , but there are more!
        // Transfer ERC1155 token from the source to the recipient.
        _transferERC1155(
            item.token,
            from,
            item.recipient,
            item.identifier,
            item.amount,
            conduitKey,
            accumulator
        );
    }

However the definition of the struct is

enum ItemType {
 // 0: ETH on mainnet, MATIC on polygon, etc.
 NATIVE,
 // 1: ERC20 items (ERC777 and ERC20 analogues could also technically work)
ERC20,
// 2: ERC721 items
ERC721,
// 3: ERC1155 items
ERC1155,
//  4: ERC721 items where a number of tokenIds are supported
ERC721_WITH_CRITERIA,
// 5: ERC1155 items where a number of ids are supported
ERC1155_WITH_CRITERIA
}   

That means you forgot two important options that can make revert your transaction for example it someone set ERC721_WITH_CRITERIA In any case this is a bad practice because the contract will assumes every number higher than 3 to be ERC1155.

Recommendation

Add proper checks in else clause.

[0age]

This is checked elsewhere, specifically when asserting that all criteria-based items have been fully resolved.

[HardlyDifficult]

The _applyCriteriaResolvers function will replace ERC721_WITH_CRITERIA with ERC721 -- downstream of this function ERC721_WITH_CRITERIA is no longer a valid value to check for.