0age
commented
2 years ago there are valid cases where a fulfiller may not actually want to accept all the offer items. Ultimately, this should be better documented at a minimum but the onus is on the fulfiller submitting the transaction to ensure that the call is properly formatted and results in the intended behavior.
HardlyDifficult
Lines of code
https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderCombiner.sol#L604
Vulnerability details
Impact
When matching and aggregating orders through
fulfillAvailableOrders/fulfillAvailableAdvancedOrders(see_fulfillAvailableAdvancedOrders),executions(kind of like transfers) are created which are then executed in_performFinalChecksAndExecuteOrders. The aggregation step sets the aggregated items' amounts to zero in theadvancedOrdersstructures and the code checks that all original consideration items are fulfilled. However, it does not check that all offer items are fulfilled. Meaning, the caller could have done a wrong aggregation on the offer items part which are then simply not transferred to the caller.In the worst case, the fulfiller sends all their consideration items to the offerer but receives none of the offer items of the orders.
Example
Alice wants to sell 2 NFTs in two orders:
Bob can call
fulfillAvailableAdvancedOrdersand aggregate the two 1 WETH consideration items to create a single execution of 2 WETH to Alice. If Bob doesn't also specify aggregations for each of the single offer NFTs, Bob will not receive them.Recommended Mitigation Steps
There does not seem to be a reason to not accept all offer items if one paid for all consideration items already. Consider protecting the fulfiller by checking that they actually received all the offer items of the "unavailable" orders, similar to the consideration item amount zero check.