LooksRare Aggregator contest - ktg's results

Lines of code

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/ERC20EnabledLooksRareAggregator.sol#L34-L36

Vulnerability details

Impact

• A user can buy NFT for free in edge cases.
• A user can withdraw all stuck tokens.

Proof of Concept

Currently the function to buy NFT with ERC20 tokens in file ERC20EnabledLooksRareAggregator.sol is implemented like this:

function execute(
        TokenTransfer[] calldata tokenTransfers,
        ILooksRareAggregator.TradeData[] calldata tradeData,
        address recipient,
        bool isAtomic
    ) external payable {
        if (tokenTransfers.length == 0) revert UseLooksRareAggregatorDirectly();
        _pullERC20Tokens(tokenTransfers, msg.sender);
        aggregator.execute{value: msg.value}(tokenTransfers, tradeData, msg.sender, recipient, isAtomic);
    }

This is TokenTransfer struct, which contains ERC20 currency and amount the buyer deposit for buying NFT.

struct TokenTransfer {
    uint256 amount; // ERC20 transfer amount
    address currency; // ERC20 transfer currency
}

Function execute will call _pullERC20Tokens and transfer ERC20 token from the caller to the aggregator. However, the tradeData.order.currency is the one used for purchase NFT, not tokenTransfer.currency, and there is no checking if these 2 currencies actually match.

struct TradeData {
        ...
        BasicOrder[] orders; // Orders to be executed by the marketplace
      ....
    }

struct BasicOrder {
  ....
  address currency; // The order's currency, address(0) for ETH
  ...
}

This can lead to user can buy NFT for free or withdraw all stuck tokens:

• Suppose there are some token A stuck in aggregator (and enough to buy some NFT)

• The buyer create tradeData.order with token A but input tokenTransfer.currency as token B

• He then call execute and receive NFT without paying (since the amount of token A stuck in aggregator pay for him), and receive full token B refund.

Tools Used

Manual source code review.

Recommended Mitigation Steps

I recommend checking inputs between transferData and tokenTransfer to make sure that tokenTransfer's tokens must contain all tokens from transferData.orders.