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

1 stars 0 forks source link

OrderValidator: The _cancel function does not validate the order status, the order will be cancelled even if the order does not exist #37

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderValidator.sol#L256-L316

Vulnerability details

Impact

In the _cancel function of the OrderValidator contract, _verifyOrderStatus is not called to verify the order status. If the user's input is incorrect, the non-existing order will be cancelled without any message, causing the user to think that the correct order has been cancelled. In particular, the orderHash will vary depending on the order of elements in offer and consideration, and users are likely to enter incorrect order parameters due to the wrong order.

Proof of Concept

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderValidator.sol#L256-L316 https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/Consideration.sol#L423-L430

Tools Used

None

Recommended Mitigation Steps

    function _cancel(OrderComponents[] calldata orders)
        internal
        returns (bool cancelled)
    {
        // Ensure that the reentrancy guard is not currently set.
        _assertNonReentrant();

        address offerer;
        address zone;

        // Skip overflow check as for loop is indexed starting at zero.
        unchecked {
            // Read length of the orders array from memory and place on stack.
            uint256 totalOrders = orders.length;

            // Iterate over each order.
            for (uint256 i = 0; i < totalOrders; ) {
               ...
                bytes32 orderHash = _deriveOrderHash(
                    OrderParameters(
                        offerer,
                        zone,
                        order.offer,
                        order.consideration,
                        order.orderType,
                        order.startTime,
                        order.endTime,
                        order.zoneHash,
                        order.salt,
                        order.conduitKey,
                        order.consideration.length
                    ),
                    order.nonce
                );
+            OrderStatus memory orderStatus = _orderStatus[orderHash];
+            _verifyOrderStatus(
+                orderHash,
+                orderStatus,
+                false,
+                true
          // Update the order status as not valid and cancelled.
          _orderStatus[orderHash].isValidated = false;
          _orderStatus[orderHash].isCancelled = true;
kartoonjoy commented 2 years ago

Per warden CCCZ, the 3 findings related to the OrderValidator are to be withdrawn. Thanks