code-423n4 / 2023-01-opensea-findings

0 stars 0 forks source link

Wrong logical condition for function _validateOrdersAndPrepareToFulfill() #58

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/OrderCombiner.sol#L431-L440

Vulnerability details

Impact

Detailed description of the impact of this finding. The logical condition is wrong for function _validateOrdersAndPrepareToFulfill().

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. The implementation of the following code is not consistent with the given documentation:

// If the first bit is set, a native offer item was encountered. If the
        // second bit is set in the error buffer, the current function is not
        // matchOrders or matchAdvancedOrders. If the value is three, both the
        // first and second bits were set; in that case, revert with an error.
        if (
            invalidNativeOfferItemErrorBuffer ==
            NonMatchSelector_InvalidErrorValue
        ) {
            _revertInvalidNativeOfferItem();
        }

We need to detect whether the value is equal to 3 to revert. The above equality comparison will not detect the first two bits.

Tools Used

Remix

Recommended Mitigation Steps

The correction is as follows:

// If the first bit is set, a native offer item was encountered. If the
        // second bit is set in the error buffer, the current function is not
        // matchOrders or matchAdvancedOrders. If the value is three, both the
        // first and second bits were set; in that case, revert with an error.
        if (invalidNativeOfferItemErrorBuffer & 3) == 3) {
            _revertInvalidNativeOfferItem();
        }
0age commented 1 year ago

the logic changed right before the contest kicked off but the comments were not properly updated to reflect the change — will share a PR once we've updated the comments!

HickupHH3 commented 1 year ago

Ok, comment issue, downgrading to QA.

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2023-01-opensea-findings/issues/63