LooksRare Aggregator contest - Rolezn's results

Lines of code

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/LooksRareAggregator.sol#L109 https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/lowLevelCallers/LowLevelETH.sol#L43-L49

Vulnerability details

Description

Currently, any user who calls execute will retrieve all of LooksRareAggregator.sol contract balance instead of the extra sent.

This happens because when _returnETHIfAny is called, it sends the entire balance of the aggregator balance:

assembly {
            if gt(selfbalance(), 0) {
                let status := call(gas(), recipient, selfbalance(), 0, 0, 0, 0)
            }
        }

Impact

Any locked ETH in the LooksRareAggregator.sol contract can be fully retrieved by anyone who calls execute that sends extra ETH.

Proof of Concept

Assume following scenarios that could cause the aggregator contract to have locked ETH: 1. User accidently sends ETH to the contract and is received by the contract's receive() fallback function 2. The _returnETHIfAny fails when sending ETH back to the user during the call process for some reason. ETH will be stuck in the aggregator contract.

Forge Test

Forge test contains two functions: 1. test_StartZeroAggregatorBalance_RetrieveETH Tests out when aggregator initial balance is 0, a user accidently sends 10 ether to aggregator and a buyer executes with 1 wei extra. 2. test_AggregatorForkBalance_RetrieveETH Forks the aggregator address and balance (as seen in comments), results in initial balance of 5.656 ether in test, _buyer will retrieve 5.656 ether + 1 wei

Filename: RetrieveEth.t.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.17;

import {IERC721} from "../../contracts/interfaces/IERC721.sol";
import {IERC1155} from "../../contracts/interfaces/IERC1155.sol";
import {LooksRareAggregator} from "../../contracts/LooksRareAggregator.sol";
import {LooksRareProxy} from "../../contracts/proxies/LooksRareProxy.sol";
import {SeaportProxy} from "../../contracts/proxies/SeaportProxy.sol";
import {ILooksRareAggregator} from "../../contracts/interfaces/ILooksRareAggregator.sol";
import {BasicOrder, TokenTransfer} from "../../contracts/libraries/OrderStructs.sol";
import {MockERC20} from "./utils/MockERC20.sol";
import {TestHelpers} from "./TestHelpers.sol";
import {TestParameters} from "./TestParameters.sol";
import {LooksRareProxyTestHelpers} from "./LooksRareProxyTestHelpers.sol";
import {SeaportProxyTestHelpers} from "./SeaportProxyTestHelpers.sol";
import {ScamRecipient} from "./utils/ScamRecipient.sol";


/**
 * @notice LooksRareAggregator execution fail scenarios
 */
contract RetrieveEthTest is
    TestParameters,
    TestHelpers,
    LooksRareProxyTestHelpers,
    SeaportProxyTestHelpers
{
    LooksRareAggregator private aggregator;

    function test_StartZeroAggregatorBalance_RetrieveETH() public {
        vm.createSelectFork(vm.rpcUrl("mainnet"), 15_282_897);

        aggregator = new LooksRareAggregator();
        LooksRareProxy looksRareProxy = new LooksRareProxy(LOOKSRARE_V1, address(aggregator));
        // Since we are forking mainnet, we have to make sure it has 0 ETH.
        vm.deal(address(looksRareProxy), 0);
        vm.deal(address(aggregator), 0); //@audit set aggregator with 0 ETH.

        aggregator.addFunction(address(looksRareProxy), LooksRareProxy.execute.selector);

        TokenTransfer[] memory tokenTransfers = new TokenTransfer[](0);
        BasicOrder[] memory validOrders = validBAYCOrders();
        BasicOrder[] memory orders = new BasicOrder[](1);
        orders[0] = validOrders[0];

        bytes[] memory ordersExtraData = new bytes[](1);
        ordersExtraData[0] = abi.encode(orders[0].price, 9550, 0, LOOKSRARE_STRATEGY_FIXED_PRICE);

        ILooksRareAggregator.TradeData[] memory tradeData = new ILooksRareAggregator.TradeData[](1);
        tradeData[0] = ILooksRareAggregator.TradeData({
            proxy: address(looksRareProxy),
            selector: LooksRareProxy.execute.selector,
            value: orders[0].price,
            maxFeeBp: 0,
            orders: orders,
            ordersExtraData: ordersExtraData,
            extraData: ""
        });
        
        //@audit Assume a user acciently sends 10 ether directly to aggregator or 10 ether is stuck in aggregator.
        vm.prank(user1);
        vm.deal(user1, 10 ether);
        payable(aggregator).call{value: 10 ether}("");
        emit log_named_uint("Balance of aggregator before execute", address(aggregator).balance);
        uint startAggregatorBalance = address(aggregator).balance;

        vm.prank(_buyer);
        vm.deal(_buyer, orders[0].price + 1 wei);
        emit log_named_uint("Balance of _buyer before execute", _buyer.balance);
        emit log_named_uint("Order price is", orders[0].price);
        emit log_named_address("Address of _buyer before execute", _buyer);

        vm.expectEmit(true, false, false, false);
        emit Sweep(_buyer);

        aggregator.execute{value: orders[0].price + 1 wei}(tokenTransfers, tradeData, address(0), _buyer, false); //@audit _buyer sends 1 additional wei, as a result will trigger _returnETHIfAny
        emit log_named_uint("Expected balance of _buyer after execute", 1 wei);
        emit log_named_uint("Balance of _buyer after execute", _buyer.balance); //@audit _buyer should have 1 wei but instad has 10 ether + 1 wei
        emit log_named_uint("Balance of aggregator after execute", address(aggregator).balance); //@audit aggregator will have 0 from initial 10 ether

        assertEq(_buyer.balance, startAggregatorBalance + 1 wei); //@audit _buyer has 10 ether + 1 wei in balance
    }

    function test_AggregatorForkBalance_RetrieveETH() public {
        vm.createSelectFork(vm.rpcUrl("mainnet"), 15_282_897);

        aggregator = new LooksRareAggregator();
        LooksRareProxy looksRareProxy = new LooksRareProxy(LOOKSRARE_V1, address(aggregator));
        // Since we are forking mainnet, we have to make sure it has 0 ETH.
        vm.deal(address(looksRareProxy), 0);

        aggregator.addFunction(address(looksRareProxy), LooksRareProxy.execute.selector);

        TokenTransfer[] memory tokenTransfers = new TokenTransfer[](0);
        BasicOrder[] memory validOrders = validBAYCOrders();
        BasicOrder[] memory orders = new BasicOrder[](1);
        orders[0] = validOrders[0];

        bytes[] memory ordersExtraData = new bytes[](1);
        ordersExtraData[0] = abi.encode(orders[0].price, 9550, 0, LOOKSRARE_STRATEGY_FIXED_PRICE);

        ILooksRareAggregator.TradeData[] memory tradeData = new ILooksRareAggregator.TradeData[](1);
        tradeData[0] = ILooksRareAggregator.TradeData({
            proxy: address(looksRareProxy),
            selector: LooksRareProxy.execute.selector,
            value: orders[0].price,
            maxFeeBp: 0,
            orders: orders,
            ordersExtraData: ordersExtraData,
            extraData: ""
        });
        
        emit log_named_address("Address of aggregator", address(aggregator)); //@audit 0xce71065d4017f316ec606fe4422e11eb2c47c246
        emit log_named_uint("Balance of aggregator before execute", address(aggregator).balance); //@audit Address holds 5.656 Ether from the fork: https://etherscan.io/address/0xce71065d4017f316ec606fe4422e11eb2c47c246

        uint startAggregatorBalance = address(aggregator).balance;

        vm.prank(_buyer);
        vm.deal(_buyer, orders[0].price + 1 wei);
        emit log_named_uint("Balance of _buyer before execute", _buyer.balance);
        emit log_named_uint("Order price is", orders[0].price);
        emit log_named_address("Address of _buyer before execute", _buyer);

        vm.expectEmit(true, false, false, false);
        emit Sweep(_buyer);

        aggregator.execute{value: orders[0].price + 1 wei}(tokenTransfers, tradeData, address(0), _buyer, false);
        emit log_named_uint("Expected balance of _buyer after execute", 1 wei);
        emit log_named_uint("Balance of _buyer after execute", _buyer.balance); //@audit _buyer should have 1 wei but instad has 5.656 ether + 1 wei
        emit log_named_uint("Balance of aggregator after execute", address(aggregator).balance); //@audit aggregator will have 0 from initial 5.656 ether

        assertEq(_buyer.balance, startAggregatorBalance + 1 wei);
    }

}

Test Results

FOUNDRY_PROFILE=local forge test --match-test test_StartZeroAggregatorBalance_RetrieveETH -vvv

[PASS] test_StartZeroAggregatorBalance_RetrieveETH() (gas: 4682818)
Logs:
  Balance of aggregator before execute: 10000000000000000000
  Balance of _buyer before execute: 81800000000000000001
  Order price is: 81800000000000000000
  Address of _buyer before execute: 0x38dee936aa3d8e6370524a7cba38c96a896d9d9f
  Expected balance of _buyer after execute: 1
  Balance of _buyer after execute: 10000000000000000001
  Balance of aggregator after execute: 0

Test result: ok. 1 passed; 0 failed; finished in 250.80ms

FOUNDRY_PROFILE=local forge test --match-test test_AggregatorForkBalance_RetrieveETH -vvv

Running 1 test for test/foundry/RetrieveETH.t.sol:RetrieveEthTest
[PASS] test_AggregatorForkBalance_RetrieveETH() (gas: 4673844)
Logs:
  Address of aggregator: 0xce71065d4017f316ec606fe4422e11eb2c47c246
  Balance of aggregator before execute: 5656000000000000000
  Balance of _buyer before execute: 81800000000000000001
  Order price is: 81800000000000000000
  Address of _buyer before execute: 0x38dee936aa3d8e6370524a7cba38c96a896d9d9f
  Expected balance of _buyer after execute: 1
  Balance of _buyer after execute: 5656000000000000001
  Balance of aggregator after execute: 0

Test result: ok. 1 passed; 0 failed; finished in 288.81ms

Recommended Mitigation Steps

Only send back the amount extra that was sent by the buyer who executed the call instead of the entire aggregator balance.

Summary<a name="Summary">

Low Risk Issues

Total: 15 contexts over 5 issues

Non-critical Issues

Total: 20 contexts over 6 issues

Low Risk Issues

<a href="#Summary">[LOW‑1]</a><a name="LOW&#x2011;1"> Unused receive() Function Will Lock Ether In Contract

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert

<ins>Proof Of Concept</ins>

220: receive() external payable {}

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/LooksRareAggregator.sol#L220

86: receive() external payable {}

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/SeaportProxy.sol#L86

<ins>Recommended Mitigation Steps</ins>

The function should call another function, otherwise it should revert

<a href="#Summary">[LOW‑2]</a><a name="LOW&#x2011;2"> Missing Contract-existence Checks Before Low-level Calls

Low-level calls return success if there is no code present at the specified address.

<ins>Proof Of Concept</ins>

88: (bool success, bytes memory returnData) = singleTradeData.proxy.delegatecall(proxyCalldata);

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/LooksRareAggregator.sol#L88

30: (bool status, ) = collection.call(

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC1155Transfer.sol#L30

52: (bool status, ) = collection.call(

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC1155Transfer.sol#L52

25: (bool status, bytes memory data) = currency.call(abi.encodeWithSelector(IERC20.approve.selector, to, amount));

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC20Approve.sol#L25

30: (bool status, bytes memory data) = currency.call(

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC20Transfer.sol#L30

51: (bool status, bytes memory data) = currency.call(abi.encodeWithSelector(IERC20.transfer.selector, to, amount));

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC20Transfer.sol#L51

27: (bool status, ) = collection.call(abi.encodeWithSelector(IERC721.transferFrom.selector, from, to, tokenId));

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC721Transfer.sol#L27

<ins>Recommended Mitigation Steps</ins>

In addition to the zero-address checks, add a check to verify that <address>.code.length > 0

<a href="#Summary">[LOW‑3]</a><a name="LOW&#x2011;3"> Missing parameter validation in constructor

Some parameters of constructors are not checked for invalid values.

<ins>Proof Of Concept</ins>

22: aggregator = ILooksRareAggregator(_aggregator);

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/ERC20EnabledLooksRareAggregator.sol#L22

38: marketplace = ILooksRareExchange(_marketplace);
39: aggregator = _aggregator;

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/LooksRareProxy.sol#L38-L39

46: marketplace = SeaportInterface(_marketplace);
47: aggregator = _aggregator;

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/SeaportProxy.sol#L46-L47

<ins>Recommended Mitigation Steps</ins>

Validate the parameters.

<a href="#Summary">[LOW‑4]</a><a name="LOW&#x2011;4"> Out of bound loop block gas limit

tradeData arrays that are provided in the execute functions in LooksRareAggregator.sol may exceed block gas limit if provided with a very large tradeData list.

<ins>Proof Of Concept</ins>

for (uint256 i; i < tradeDataLength; ) {

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/LooksRareAggregator.sol#L69

Recommended Mitigation Steps

Consider introducing max limits on the tradeData that can be sent in the execution function.

<a href="#Summary">[LOW‑5]</a><a name="LOW&#x2011;5"> Can't remove old proxy fee data

It is not possible to remove old proxy fee data should it accumlate a large mapping of proxy fee data.

<ins>Proof Of Concept</ins>

mapping(address => FeeData) private _proxyFeeData;

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/LooksRareAggregator.sol#L46

Recommended Mitigation Steps

Add a function to delete old _proxyFeeData

Non Critical Issues

<a href="#Summary">[NC‑1]</a><a name="NC&#x2011;1"> Critical Changes Should Use Two-step Procedure

The critical procedures should be two step process.

See similar findings in previous Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#2-critical-changes-should-use-two-step-procedure

Specifically setERC20EnabledLooksRareAggregator can only be set once as described, it is important that it is set correctly.

<ins>Proof Of Concept</ins>

120: function setERC20EnabledLooksRareAggregator(address _erc20EnabledLooksRareAggregator) external onlyOwner {

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/LooksRareAggregator.sol#L120

153: function setFee(

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/LooksRareAggregator.sol#L153

<ins>Recommended Mitigation Steps</ins>

Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.

<a href="#Summary">[NC‑2]</a><a name="NC&#x2011;2"> Event Is Missing Indexed Fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields).

Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

<ins>Proof Of Concept</ins>

event FeeUpdated(address proxy, uint256 bp, address recipient);

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/interfaces/ILooksRareAggregator.sol#L44

<a href="#Summary">[NC‑3]</a><a name="NC&#x2011;3"> Implementation contract may not be initialized

OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5

<ins>Proof Of Concept</ins>

constructor(address _aggregator) {

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/ERC20EnabledLooksRareAggregator.sol#L21

constructor(address _marketplace, address _aggregator) {

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/LooksRareProxy.sol#L37

constructor(address _marketplace, address _aggregator) {

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/SeaportProxy.sol#L45

<a href="#Summary">[NC‑4]</a><a name="NC&#x2011;4"> Avoid Floating Pragmas: The Version Should Be Locked

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

<ins>Proof Of Concept</ins>

Found usage of floating pragmas ^0.8.14 of Solidity in [OwnableTwoSteps.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/OwnableTwoSteps.sol#L2

Found usage of floating pragmas ^0.8.14 of Solidity in [ReentrancyGuard.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/ReentrancyGuard.sol#L2

Found usage of floating pragmas ^0.8.14 of Solidity in [SignatureChecker.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/SignatureChecker.sol#L2

Found usage of floating pragmas ^0.8.0 of Solidity in [IERC20.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/interfaces/IERC20.sol#L2

Found usage of floating pragmas ^0.8.0 of Solidity in [IERC721.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/interfaces/IERC721.sol#L2

Found usage of floating pragmas ^0.8.14 of Solidity in [LowLevelERC1155Transfer.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC1155Transfer.sol#L2

Found usage of floating pragmas ^0.8.14 of Solidity in [LowLevelERC20Approve.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC20Approve.sol#L2

Found usage of floating pragmas ^0.8.14 of Solidity in [LowLevelERC20Transfer.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC20Transfer.sol#L4

Found usage of floating pragmas ^0.8.14 of Solidity in [LowLevelERC721Transfer.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelERC721Transfer.sol#L2

Found usage of floating pragmas ^0.8.14 of Solidity in [LowLevelETH.sol]

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/lowLevelCallers/LowLevelETH.sol#L2

<a href="#Summary">[NC‑5]</a><a name="NC&#x2011;5"> Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length Reference: https://docs.soliditylang.org/en/v0.8.10/style-guide.html#maximum-line-length

<ins>Proof Of Concept</ins>

35: // An arbitrary 32-byte value that will be supplied to the zone when fulfilling restricted orders that the zone can utilize when making a determination on whether to authorize the order

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/SeaportProxy.sol#L35

<a href="#Summary">[NC‑6]</a><a name="NC&#x2011;6"> Duplicate imports

The contract LookRareAggregator.sol contains multiple duplicate imports

<ins>Proof Of Concept</ins>

File: LooksRareAggregator.sol

9: import {LooksRareProxy} from "./proxies/LooksRareProxy.sol";
15: import {LooksRareProxy} from "./proxies/LooksRareProxy.sol";

11: import {TokenRescuer} from "./TokenRescuer.sol";
17: import {TokenRescuer} from "./TokenRescuer.sol";

12: import {TokenReceiver} from "./TokenReceiver.sol";
16: import {TokenReceiver} from "./TokenReceiver.sol";

10: import {BasicOrder, TokenTransfer} from "./libraries/OrderStructs.sol";
14: import {BasicOrder, FeeData, TokenTransfer} from "./libraries/OrderStructs.sol";

Summary<a name="Summary">

Gas Optimizations

Total: 355 contexts over 6 issues

Gas Optimizations

<a href="#Summary">[GAS‑1]</a><a name="GAS&#x2011;1"> Empty Blocks Should Be Removed Or Emit Something

The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting. If the contract is meant to be extended, the contract should be abstract and the function signatures be added without any default implementation.

If the block is an empty if-statement block to avoid doing subsequent checks in the else-if/else conditions, the else-if/else conditions should be nested under the negation of the if-statement, because they involve different classes of checks, which may lead to the introduction of errors when the code is later modified (if(x){}else if(y){...}else{...} => if(!x){if(y){...}else{...}})

<ins>Proof Of Concept</ins>

220: receive() external payable {}

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/LooksRareAggregator.sol#L220

132: } catch {}

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/LooksRareProxy.sol#L132

86: receive() external payable {}

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/SeaportProxy.sol#L86

217: } catch {}

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/SeaportProxy.sol#L217

<a href="#Summary">[GAS‑2]</a><a name="GAS&#x2011;2"> Use calldata instead of memory for function parameters

In some cases, having function arguments in calldata instead of memory is more optimal.

Consider the following generic example:

contract C {
	function add(uint[] memory arr) external returns (uint sum) {
		uint length = arr.length;
		for (uint i = 0; i < arr.length; i++) {
		    sum += arr[i];
		}
	}
}

In the above example, the dynamic array arr has the storage location memory. When the function gets called externally, the array values are kept in calldata and copied to memory during ABI decoding (using the opcode calldataload and mstore). And during the for loop, arr[i] accesses the value in memory using a mload. However, for the above example this is inefficient. Consider the following snippet instead:

contract C {
	function add(uint[] calldata arr) external returns (uint sum) {
		uint length = arr.length;
		for (uint i = 0; i < arr.length; i++) {
		    sum += arr[i];
		}
	}
}

In the above snippet, instead of going via memory, the value is directly read from calldata using calldataload. That is, there are no intermediate memory operations that carries this value.

Gas savings: In the former example, the ABI decoding begins with copying value from calldata to memory in a for loop. Each iteration would cost at least 60 gas. In the latter example, this can be completely avoided. This will also reduce the number of instructions and therefore reduces the deploy time cost of the contract.

In short, use calldata instead of memory if the function argument is only read.

Note that in older Solidity versions, changing some function arguments from memory to calldata may cause "unimplemented feature error". This can be avoided by using a newer (0.8.*) Solidity compiler.

Examples Note: The following pattern is prevalent in the codebase:

function f(bytes memory data) external {
	(...) = abi.decode(data, (..., types, ...));
}

Here, changing to bytes calldata will decrease the gas. The total savings for this change across all such uses would be quite significant.

<ins>Proof Of Concept</ins>

function onERC1155BatchReceived(
        address,
        address,
        uint256[] memory,
        uint256[] memory,
        bytes memory
    ) external virtual returns (bytes4) {

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/TokenReceiver.sol#L24

<a href="#Summary">[GAS‑3]</a><a name="GAS&#x2011;3"> Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract's gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed

<ins>Proof Of Concept</ins>

188: uint120 numerator;

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/libraries/seaport/ConsiderationStructs.sol#L188

189: uint120 denominator;

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/libraries/seaport/ConsiderationStructs.sol#L189

31: uint120 numerator;

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/SeaportProxy.sol#L31

32: uint120 denominator;

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/proxies/SeaportProxy.sol#L32

<a href="#Summary">[GAS‑4]</a><a name="GAS&#x2011;4"> Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

<ins>Proof Of Concept</ins>

Applies to all in-scope contracts.

<a href="#Summary">[GAS‑5]</a><a name="GAS&#x2011;5"> Use uint256(1)/uint256(2) instead for true and false boolean states

If you don't use boolean for storage you will avoid Gwarmaccess 100 gas. In addition, state changes of boolean from true to false can cost up to ~20000 gas rather than uint256(2) to uint256(1) that would cost significantly less.

<ins>Proof Of Concept</ins>

45: mapping(address => mapping(bytes4 => bool)) private _proxyFunctionSelectors;

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/LooksRareAggregator.sol#L45

133: _proxyFunctionSelectors[proxy][selector] = true;

https://github.com/code-423n4/2022-11-looksrare/tree/main/contracts/LooksRareAggregator.sol#L133

<a href="#Summary">[GAS‑6]</a><a name="GAS&#x2011;6"> Duplicate imports

The contract LookRareAggregator.sol contains multiple duplicate imports.

<ins>Proof Of Concept</ins>

File: LooksRareAggregator.sol

9: import {LooksRareProxy} from "./proxies/LooksRareProxy.sol";
15: import {LooksRareProxy} from "./proxies/LooksRareProxy.sol";

11: import {TokenRescuer} from "./TokenRescuer.sol";
17: import {TokenRescuer} from "./TokenRescuer.sol";

12: import {TokenReceiver} from "./TokenReceiver.sol";
16: import {TokenReceiver} from "./TokenReceiver.sol";

10: import {BasicOrder, TokenTransfer} from "./libraries/OrderStructs.sol";
14: import {BasicOrder, FeeData, TokenTransfer} from "./libraries/OrderStructs.sol";