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

1 stars 0 forks source link

A Malicious offerer can temporarily cancel the orders to deny the fulfillments #119

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderValidator.sol#L48 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/OrderValidator.sol#L104

Vulnerability details

Impact

When creating an offer, the offerer should have sufficient balance of all offered items. And the offerer should have sufficient approvals set for the Seaport contract for all offered items. So the fulfiller can successfully fulfill the order.

However, a malicious offerer can use approval to temporarily cancel an order and re-open it at any time. Normally, an offerer should call cancel() or incrementNonce() to cancel an order. But a malicious offer can reset the approval to make the fulfillment fail. And set the approval back when he or she wants to open the order. It breaks the rule of order cancellation.

This vulnerability can apply in many situations. For example, when a fulfiller tries to fulfill a group of orders, an offerer of one of the orders can reset the approval ahead of the fulfillments to deny the fulfillment. And set the approval back after the fulfillment fails. Then it is hard for the fulfiller to find out the malicious order. Moreover, the malicious offerer can fulfill other orders ahead of the fulfiller if he or she can profit from those orders.

Proof of Concept

Tools Used

None

Recommended Mitigation Steps

When doing fulfillment, Seaport can check the approval in _validateOrderAndUpdateStatus() or _validateAndFulfillBasicOrder(). Also, fulfillers can use revertOnInsufficiency variable to skip or revert on any order which has insufficient allowance.

0age commented 2 years ago

This would add appreciable overhead to the base case and can still be accomplished by any fulfiller that desires these extra checks by using a router that asserts sufficient balance and allowance prior to performing the call.

HardlyDifficult commented 2 years ago

Merging with the warden's QA report https://github.com/code-423n4/2022-05-opensea-seaport-findings/issues/123