code-423n4 / 2022-11-looksrare-findings

0 stars 0 forks source link

Orders take an array of tokenIds & amounts, but only executes for the first item #195

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/libraries/OrderStructs.sol#L6-L17 https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/LooksRareProxy.sol#L74 https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/SeaportProxy.sol#L248

Vulnerability details

Impact

In OrderStructs.sol, the struct BasicOrder stores an array of uint256 to store multiple tokenIds. However the seaport/looksrare proxies arent set up to handle multiple tokenIds within a single order and can only execute for the 1st id in the order.

Proof of Concept

SeaportProxy.sol:

function _populateParameters(BasicOrder calldata order, OrderExtraData memory orderExtraData)
        private
        pure
        returns (OrderParameters memory parameters)
    {
        ...
        // Lines 248-249
        offer[0].identifierOrCriteria = order.tokenIds[0];
        uint256 amount = order.amounts[0];
        ...
    }

LooksRareProxy.sol:

function execute(
        BasicOrder[] calldata orders,
        bytes[] calldata ordersExtraData,
        bytes memory,
        address recipient,
        bool isAtomic,
        uint256,
        address
    ) external payable override {
        ...
        // Lines 74-76
        makerAsk.tokenId = order.tokenIds[0];
        makerAsk.price = orderExtraData.makerAskPrice;
        makerAsk.amount = order.amounts[0];
        ...
    }

Tools Used

forge

Recommended Mitigation Steps

Either change uint256[] to uint256 in BasicOrder struct, or implement a loop in the market proxies to loop over multiple tokenids and amounts with an order

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

c4-sponsor commented 1 year ago

0xhiroshi marked the issue as sponsor disputed

0xhiroshi commented 1 year ago

this is intentional, we need to support marketplaces that support batch orders in the future while maintaining the same interface

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2022-11-looksrare-findings/issues/196