Restricted Trades Vulnerable to Stolen Items Being Traded

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-opensea-seaport/blob/7c082637d9273deb42e551c74a578151cefd8448/contracts/lib/OrderFulfiller.sol#L75-L127

Vulnerability details

Impact

The current contract does not check for stolen items. Traditionally, Opensea has frozen items that are stolen based on its stolen item policy. It was possible to do so on Opensea at the UI level.

However, with Seaport, anyone can list a restricted trade for a legit item and this trade can be fulfilled via the contract with a stolen item. Hiding at the UI level won't work because the listing signatures are for legit items and are only fulfilled by stolen items (via the contract).

This means that when Seaport is launched, there may be hundreds of trades of stolen items on the platform that could cost Opensea's users (and also possibly Opensea for compensating them) millions of dollars - anyone who holds a locked item would prefer to immediately trade it for any legit item.

Proof of Concept

Let's say someone lists a restricted order that accepts any bored ape NFT for the offerer's Moonbirds grail NFT. The consideration item is at the collection level, which means that identifierOrCriteria is set to zero. This means that if the zone check passes, any stolen item can be traded.

The same concept can be applied for OFAC sanctioned addresses by the US Treasury Department’s Office of Foreign Assets Control. Per the US treasury website: "U.S. persons (and persons otherwise subject to OFAC jurisdiction) must ensure that they block the property and interests in property of persons named on OFAC’s SDN List or any entity owned in the aggregate, directly or indirectly, 50 percent or more by one or more blocked persons, and that they do not engage in trade or other transactions with such persons."

While it can be argued that zone contracts themselves can have these checks included, each and every zone having a stolen item check (and logic + data to verify these) would be extremely cumbersome and expensive (in terms of gas to upload that data). It is therefore, practically infeasible to incorporate these checks at the zone level and must be considered at a more central level (applicable to all item trades)

It is also interesting to note that zones currently do not have the capability to block OFAC sanctioned transactions as only the offerer or msg.sender (e.g. transaction origin in matchOrders) is sent as a parameter and not the fulfiller.

Tools Used

Visual Studio Code

Recommended Mitigation Steps

It is recommended to deploy another contract (ItemValidator) that checks for stolen items and OFAC sanctioned addresses, and the Seaport contract must call the ItemValidator contract to validate both consideration and offered items (for the stolen check), along with offerer and recipient addresses (for the OFAC sanctions check).

[0age]

This behavior can easily be implemented using zones, and moreover is not a code-level issue.

[HardlyDifficult]

As both the warden and the sponsor points out, the contract can support this via Zone contracts. A marketplace could choose to only surface orders using such a Zone where appropriate in order to remain complaint. Zone contracts could also leverage a shared contract in order to avoid the gas and maintenance burden when unique Zone implementations are required.

Closing this as invalid since it amounts to a recommendation on how a website chooses to leverage this protocol.