code423n4 commented 1 year ago

Lines of code

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/Assertions.sol#L77

Vulnerability details

Impact

In the _assertConsiderationLengthIsNotLessThanOriginalConsiderationLength function, the _revertMissingOriginalConsiderationItems() function is called if the supplied consideration item total is less than the original consideration item total. However, there is no indication of what this function does or if it reverts the transaction with a proper error message.

Proof of Concept

An ex where this bug could cause an issue:

There is a smart contract that allows users to sell a digital item on an auction. The original item has 3 different files and the user can only sell the item if all 3 files are included in the auction. The smart contract is using the Assertions contract to make sure that the user includes all 3 files in the auction.

A user creates an auction for an item and sets the original item total to 3. The user then sends the transaction to the smart contract to start the auction. Another user sees the auction and wants to buy the item, but the seller only included 2 of the 3 files. The buyer sends the transaction to the smart contract with the correct amount of ether and the 2 files that the seller provided.

The smart contract checks to see if the total number of items supplied is less than the original number of items, in this case 2 < 3, so the _revertMissingOriginalConsiderationItems() function is called. However, we don't know what this function does, it could just cancel the transaction or it could also send an error message to the user. In this case, the buyer would not know why the transaction failed and would not know what they need to do to fix the issue.

Tools Used

Recommended Mitigation Steps

0age commented 1 year ago

contested; supplying fewer consideration items than the offerer originally intended should indeed revert

HickupHH3 commented 1 year ago

However, there is no indication of what this function does or if it reverts the transaction with a proper error message.

/**
 * @dev Reverts execution with a "MissingOriginalConsiderationItems" error
 *      message.
 */
function _revertMissingOriginalConsiderationItems() pure {
    assembly {
        // Store left-padded selector with push4 (reduces bytecode),
        // mem[28:32] = selector
        mstore(0, MissingOriginalConsiderationItems_error_selector)

        // revert(abi.encodeWithSignature(
        //     "MissingOriginalConsiderationItems()"
        // ))
        revert(
            Error_selector_offset,
            MissingOriginalConsiderationItems_error_length
        )
    }
}

c4-judge commented 1 year ago

HickupHH3 marked the issue as unsatisfactory: Insufficient quality

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