Fulfillers may lose offer items

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L472-L495

Vulnerability details

Impact

When a fulfiller calls fulfillAvailableOrders or fulfillAvailableAdvancedOrders, the fulfiller may lose offer items if he or she misses some items in offerFulfillments. The transaction will succeed but the fulfiller will not be able to get the offers.

Proof of Concept

In fulfillAvailableOrders and fulfillAvailableAdvancedOrders, it will call _executeAvailableFulfillments to fulfill groups of validated orders. The function will iterate over each offer fulfillment: https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderCombiner.sol#L472-L495

        // Retrieve length of offer fulfillments array and place on the stack.
        uint256 totalOfferFulfillments = offerFulfillments.length;

        ...

            // Iterate over each offer fulfillment.
            for (uint256 i = 0; i < totalOfferFulfillments; ++i) {
                /// Retrieve the offer fulfillment components in question.
                FulfillmentComponent[] memory components = (
                    offerFulfillments[i]
                );

                // Derive aggregated execution corresponding with fulfillment.
                Execution memory execution = _aggregateAvailable(
                    advancedOrders,
                    Side.OFFER,
                    components,
                    fulfillerConduitKey
                );

                // If offerer and recipient on the execution are the same...
                if (execution.item.recipient == execution.offerer) {
                    // increment total filtered executions.
                    totalFilteredExecutions += 1;
                } else {
                    // Otherwise, assign the execution to the executions array.
                    executions[i - totalFilteredExecutions] = execution;
                }
            }

If the fulfiller indicates wrong offerFulfillments (or missing some offer items to attempt to aggregate), the protocol will not transfer these offer items to the fulfiller. The transaction will succeed and the fulfiller will lose these offer items because the protocol only ensures all considerations are met (in _performFinalChecksAndExecuteOrders) but it doesn't ensure all offers should also be met.

Tools Used

None

Recommended Mitigation Steps

In fulfillAvailableOrders and fulfillAvailableAdvancedOrders, it should also check that each offer item for an arbitrary number of fulfilled orders has been met before transferring items, or the protocol directly transfers remaining offer items to the fulfiller.

[0age]

This is a legitimate observation and merits better documentation; however, there are cases where a fullfiller does not actually want all offer items on an order (particularly when there are "poison pill" offer items that have malicious transfer hooks or something to that effect). Ultimately, just as an offerer is expected to properly structure the payload when creating and signing an order, a fulfiller is expected to properly structure the payload when fulfilling and executing an order.

[HardlyDifficult]

Agree with the sponsor that there are use cases where a subset of offer items is desirable. Per the documentation point, lowering this to a QA submission and grouping with the warden's QA report #123