LooksRare Aggregator contest - zaskoh's results

Lines of code

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/LooksRareAggregator.sol#L108-L109

Vulnerability details

Impact

Every user can sweep all the ETH or ERC20-Tokens that are probably stuck in the LooksRareAggregator.
If a user by mistake send ETH or an ERC20 to the LooksRareAggregator it is "stuck" here. For this scenario the owner has the possibility to use the TokenRescuer.rescueETH and TokenRescuer.rescueERC20 functions to help the user get back his eth / erc20.
The problem is, that every user or bot can just monitor the contract and as soon as there is eth or an erc20 token it in sweep all the assets from the LooksRareAggregator and so make the rescue functions useless.

File: contracts/LooksRareAggregator.sol
107:         // @audit-info just make a valid trade via ERC20EnabledLooksRareAggregator to get all the additional ERC20-Tokens out
108:         if (tokenTransfersLength > 0) _returnERC20TokensIfAny(tokenTransfers, originator);
109:         // @audit-info just make a valid trade through ERC20EnabledLooksRareAggregator or directly on Contract with tokenTransfersLength == 0 to get all the eth out
110:         _returnETHIfAny(originator);

Proof of Concept

The following test prove that with a valid transaction a user can sweep all the eth / erc20 tokens that are stuck in the contract.

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

import {IERC20} from "../../contracts/interfaces/IERC20.sol";
import {IERC721} from "../../contracts/interfaces/IERC721.sol";
import {OwnableTwoSteps} from "../../contracts/OwnableTwoSteps.sol";
import {SeaportProxy} from "../../contracts/proxies/SeaportProxy.sol";
import {ERC20EnabledLooksRareAggregator} from "../../contracts/ERC20EnabledLooksRareAggregator.sol";
import {LooksRareAggregator} from "../../contracts/LooksRareAggregator.sol";
import {IProxy} from "../../contracts/interfaces/IProxy.sol";
import {ILooksRareAggregator} from "../../contracts/interfaces/ILooksRareAggregator.sol";
import {BasicOrder, FeeData, TokenTransfer} from "../../contracts/libraries/OrderStructs.sol";
import {TestHelpers} from "./TestHelpers.sol";
import {TestParameters} from "./TestParameters.sol";
import {SeaportProxyTestHelpers} from "./SeaportProxyTestHelpers.sol";

contract SweepEthAndErc20VulTest is TestParameters, TestHelpers, SeaportProxyTestHelpers {
    LooksRareAggregator private aggregator;
    ERC20EnabledLooksRareAggregator private erc20EnabledAggregator;
    SeaportProxy private seaportProxy;

    function setUp() public {
        vm.createSelectFork(vm.rpcUrl("mainnet"), 15_491_323);

        aggregator = new LooksRareAggregator();
        erc20EnabledAggregator = new ERC20EnabledLooksRareAggregator(address(aggregator));
        seaportProxy = new SeaportProxy(SEAPORT, address(aggregator));
        aggregator.addFunction(address(seaportProxy), SeaportProxy.execute.selector);        

        aggregator.approve(SEAPORT, USDC, type(uint256).max);
        aggregator.setFee(address(seaportProxy), 250, _protocolFeeRecipient);
        aggregator.setERC20EnabledLooksRareAggregator(address(erc20EnabledAggregator));
        
        // Clear eth and set clean balances
        deal(USDC, _buyer, INITIAL_USDC_BALANCE);
        vm.deal(_buyer, 200 ether);        
        vm.deal(address(aggregator), 0);
        deal(USDC, address(aggregator), 0);
        vm.deal(address(seaportProxy), 0);
        vm.deal(address(erc20EnabledAggregator), 0);
    }

    function testExecuteWithCleanLooksRareAggregatorState() public asPrankedUser(_buyer) {

        uint256 totalPrice = _runTrade();

        assertEq(IERC721(BAYC).balanceOf(_buyer), 2);
        assertEq(IERC721(BAYC).ownerOf(9948), _buyer);
        assertEq(IERC721(BAYC).ownerOf(8350), _buyer);
        assertEq(IERC20(USDC).balanceOf(_buyer), INITIAL_USDC_BALANCE - totalPrice);
        assertEq(address(_buyer).balance, 200 ether);
    }

    function testExecuteWithUserFoundsStuckInLooksRareAggregatorStateToBeRemoved() asPrankedUser(_buyer) public {        
        // send eth and usdc to LooksRareAggregator to simulate user mistake
        vm.deal(address(aggregator), 10 ether);
        deal(USDC, address(aggregator), 10 ether);

        assertEq(address(_buyer).balance, 200 ether);

        uint256 totalPrice = _runTrade();

        assertEq(IERC721(BAYC).balanceOf(_buyer), 2);
        assertEq(IERC721(BAYC).ownerOf(9948), _buyer);
        assertEq(IERC721(BAYC).ownerOf(8350), _buyer);
        
        // We now have more eth than we started!
        assertEq(address(_buyer).balance, 210 ether);
        // And also we sweeped all the USDC that was already in LooksRareAggregator
        assertEq(IERC20(USDC).balanceOf(_buyer), INITIAL_USDC_BALANCE - totalPrice + 10 ether);
    }

    function _runTrade() private returns (uint256 totalPrice) {
        ILooksRareAggregator.TradeData[] memory tradeData = _generateTradeData();
        totalPrice = (tradeData[0].orders[0].price * 10250) /
            10000 +
            (tradeData[0].orders[1].price * 10250) /
            10000;
        IERC20(USDC).approve(address(erc20EnabledAggregator), totalPrice);

        TokenTransfer[] memory tokenTransfers = new TokenTransfer[](1);
        tokenTransfers[0].currency = USDC;
        tokenTransfers[0].amount = totalPrice;

        erc20EnabledAggregator.execute(tokenTransfers, tradeData, _buyer, false);
    }

    function _generateTradeData() private view returns (ILooksRareAggregator.TradeData[] memory) {
        BasicOrder memory orderOne = validBAYCId9948Order();
        BasicOrder memory orderTwo = validBAYCId8350Order();
        BasicOrder[] memory orders = new BasicOrder[](2);
        orders[0] = orderOne;
        orders[1] = orderTwo;

        bytes[] memory ordersExtraData = new bytes[](2);
        {
            bytes memory orderOneExtraData = validBAYCId9948OrderExtraData();
            bytes memory orderTwoExtraData = validBAYCId8350OrderExtraData();
            ordersExtraData[0] = orderOneExtraData;
            ordersExtraData[1] = orderTwoExtraData;
        }

        bytes memory extraData = validMultipleItemsSameCollectionExtraData();
        ILooksRareAggregator.TradeData[] memory tradeData = new ILooksRareAggregator.TradeData[](1);
        tradeData[0] = ILooksRareAggregator.TradeData({
            proxy: address(seaportProxy),
            selector: SeaportProxy.execute.selector,
            value: 0,
            maxFeeBp: 250,
            orders: orders,
            ordersExtraData: ordersExtraData,
            extraData: extraData
        });

        return tradeData;
    }
}

Tools Used

forge test --match-contract=SweepEthAndErc20VulTest

Recommended Mitigation Steps

One possible way could be to track the current eth-balance and erc20-balances in a mapping and adapt the values depending on the trade data. As the LooksrareProxy and SeaportProxy are called via delegatecall it is possible to track the values, BUT in an exchange of lot of gas wasted.
Maybe it's better to just add the information in the NatSpec Docs of LooksRareAggregator and tell that if any tokens / eth are sent by mistake they will be lost.

Solidity pragma versioning

Solidity pragma versioning should be exactly same in all contracts. Currently there is a mix in versions. All Contracts and Interfaces can have pragma solidity 0.8.17;

Change:

File: contracts/interfaces/IERC20.sol
2: pragma solidity ^0.8.0;

File: contracts/interfaces/IERC721.sol
2: pragma solidity ^0.8.0;

File: contracts/interfaces/IERC1155.sol
2: pragma solidity ^0.8.0;

File: contracts/interfaces/IERC1271.sol
2: pragma solidity ^0.8.0;

File: contracts/interfaces/IOwnableTwoSteps.sol
2: pragma solidity ^0.8.14;

File: contracts/interfaces/IReentrancyGuard.sol
2: pragma solidity ^0.8.0;

File: contracts/interfaces/ISignatureChecker.sol
2: pragma solidity ^0.8.0;

File: contracts/interfaces/IWETH.sol
2: pragma solidity >=0.5.0;

File: contracts/lowLevelCallers/LowLevelERC20Approve.sol
2: pragma solidity ^0.8.14;

File: contracts/lowLevelCallers/LowLevelERC20Transfer.sol
4: pragma solidity ^0.8.14;

File: contracts/lowLevelCallers/LowLevelERC721Transfer.sol
2: pragma solidity ^0.8.14;

File: contracts/lowLevelCallers/LowLevelERC1155Transfer.sol
2: pragma solidity ^0.8.14;

File: contracts/lowLevelCallers/LowLevelETH.sol
2: pragma solidity ^0.8.14;

File: contracts/OwnableTwoSteps.sol
2: pragma solidity ^0.8.14;

File: contracts/ReentrancyGuard.sol
2: pragma solidity ^0.8.14;

File: contracts/SignatureChecker.sol
2: pragma solidity ^0.8.14;

erc20EnabledLooksRareAggregator in contracts/LooksRareAggregator.sol is not set in constructor

As the ERC20EnabledLooksRareAggregator is using the address of LooksRareAggregator as an immutable variable it is not possible to first deploy the ERC20EnabledLooksRareAggregator. This also means, that ERC20EnabledLooksRareAggregator is only usable as soon as LooksRareAggregator opened the address via setERC20EnabledLooksRareAggregator. Until that time the function will always revert if tokenTransfersLength is > 0.

_setupDelayForRenouncingOwnership not implemented in constructor as suggested in comment

File: contracts/OwnableTwoSteps.sol
122:      * @dev This function is expected to be included in the constructor of the contract that inherits this contract.
123:      *      If it is not set, there is no timelock to renounce the ownership.

The comment suggest to implement this function in the constructor of the contracts that inherits.
contracts/TokenRescuer.sol doesn't implement the function in it's constructor, so the timelock is always 0 here.

Unnecessary Imports

Some contracts have unnecessary imports that are never used and decrease readability

File: contracts/LooksRareAggregator.sol
09: import {LooksRareProxy} from "./proxies/LooksRareProxy.sol"; //  unnecessary import => imported on Line 15 again
10: import {BasicOrder, TokenTransfer} from "./libraries/OrderStructs.sol"; //  unnecessary import => imported on Line 14 again
11: import {TokenRescuer} from "./TokenRescuer.sol"; //  unnecessary import => imported on Line 17 again
12: import {TokenReceiver} from "./TokenReceiver.sol"; // unnecessary import => imported on Line 16 again
14: import {BasicOrder, FeeData, TokenTransfer} from "./libraries/OrderStructs.sol"; // unnecessary import BasicOrder

File: contracts/lowLevelCallers/LowLevelETH.sol
4: import {IWETH} from "../interfaces/IWETH.sol"; // unnecessary import 

File: contracts/proxies/LooksRareProxy.sol
05: import {IERC721} from "../../contracts/interfaces/IERC721.sol"; // unnecessary import
06: import {IERC1155} from "../../contracts/interfaces/IERC1155.sol"; // unnecessary import
11: import {BasicOrder, FeeData} from "../libraries/OrderStructs.sol"; // unnecessary import FeeData

File: contracts/proxies/SeaportProxy.sol
4: import {IERC20} from "../interfaces/IERC20.sol"; // unnecessary import
5: import {CollectionType} from "../libraries/OrderEnums.sol"; // unnecessary import
6: import {BasicOrder, FeeData} from "../libraries/OrderStructs.sol"; // unnecessary import FeeData

Add missing documentation

The contracts are very well documented and clear to read. Nearly every function and contract have complete natspec comments. Only a few are missing and could be added for a better overview.

File: contracts/ERC20EnabledLooksRareAggregator.sol
39:     function _pullERC20Tokens(TokenTransfer[] calldata tokenTransfers, address source) private {
// missing comments for function

File: contracts/TokenReceiver.sol
4: abstract contract TokenReceiver {
// missing comments for all functions

File: contracts/proxies/LooksRareProxy.sol
107:     function _executeSingleOrder(
// missing comments for function

File: contracts/proxies/SeaportProxy.sol
088:     function _executeAtomicOrders(
136:     function _handleFees(
166:     function _transferFee(
178:     function _executeNonAtomicOrders(
227:     function _populateParameters(BasicOrder calldata order, OrderExtraData memory orderExtraData)
// missing comments for function

File: contracts/SignatureChecker.sol
51:     /**
52:      * @notice Recover the signer of a signature (for EOA)
53:      * @param hash Hash of the signed message
54:      * @param signature Bytes containing the signature (64 or 65 bytes)
55:      */
56:     function _recoverEOASigner(bytes32 hash, bytes memory signature) internal pure returns (address signer) {
// missing @return address signer comment

File: contracts/LooksRareAggregator.sol
148:     /**
149:      * @param proxy Proxy to apply the fee to
150:      * @param bp Fee basis point
151:      * @param recipient Fee recipient
152:      */
153:     function setFee(
// missing @notice

File: contracts/LooksRareAggregator.sol
179:     /**
180:      * @param proxy The marketplace proxy's address
181:      * @param selector The marketplace proxy's function selector
182:      * @return Whether the marketplace proxy's function can be called from the aggregator
183:      */
184:     function supportsProxyFunction(address proxy, bytes4 selector) external view returns (bool) {
// missing @notice

File: contracts/LooksRareAggregator.sol
222:     function _encodeCalldataAndValidateFeeBp(
241:     function _returnERC20TokensIfAny(TokenTransfer[] calldata tokenTransfers, address recipient) private {
// missing comments for function

File: contracts/lowLevelCallers/LowLevelETH.sol
40:     /**
41:      * @notice Return ETH back to the designated sender if any ETH is left in the payable call.
42:      */
// missing @param recipient

use unchecked and payable in contracts/TokenRescuer.sol

TokenRescuer is protected from onlyOwner and so we can add the payable to save some gas. Also you can use unchecked{} to save even more gas for ETH and ERC20 transfers.

For rescueETH

-    function rescueETH(address to) external onlyOwner {
-        uint256 withdrawAmount = address(this).balance - 1;
-        if (withdrawAmount == 0) revert InsufficientAmount();
-        _transferETH(to, withdrawAmount);
+    function rescueETH(address to) external payable onlyOwner {
+        uint256 withdrawAmount = address(this).balance;
+        if (withdrawAmount < 2) revert InsufficientAmount();
+        unchecked{_transferETH(to, withdrawAmount - 1);}
    }

For rescueERC20

-    function rescueERC20(address currency, address to) external onlyOwner {
-        uint256 withdrawAmount = IERC20(currency).balanceOf(address(this)) - 1;
-        if (withdrawAmount == 0) revert InsufficientAmount();
-        _executeERC20DirectTransfer(currency, to, withdrawAmount);
+    function rescueERC20(address currency, address to) external payable onlyOwner {
+        uint256 withdrawAmount = IERC20(currency).balanceOf(address(this));
+        if (withdrawAmount < 2) revert InsufficientAmount();
+        unchecked{_executeERC20DirectTransfer(currency, to, withdrawAmount - 1);}
    }

Savings (forge snapshot --diff):

testRescueERC20NotOwner() (gas: -24 (-0.003%))
testRescueERC20NotOwner() (gas: -24 (-0.003%))
testRescueERC20NotOwner() (gas: -24 (-0.003%))
testRescueERC20() (gas: -96 (-0.010%))
testRescueERC20() (gas: -96 (-0.010%))
testRescueERC20() (gas: -96 (-0.010%))
testRescueERC20InsufficientAmount() (gas: -102 (-0.011%))
testRescueERC20InsufficientAmount() (gas: -102 (-0.011%))
testExecuteOrdersLengthMismatch() (gas: -83 (-0.117%))
testExecuteZeroOrders() (gas: -82 (-0.160%))
testRescueETHNotOwner() (gas: -24 (-0.197%))
testRescueETHNotOwner() (gas: -24 (-0.197%))
testRescueETHNotOwner() (gas: -24 (-0.198%))
testExecuteReentrancy() (gas: -14825 (-0.210%))
testRescueETH() (gas: -95 (-0.219%))
testRescueETH() (gas: -95 (-0.219%))
testRescueETH() (gas: -95 (-0.220%))
testExecuteThroughAggregatorTwoOrdersAtomic() (gas: -14825 (-0.260%))
testExecuteThroughAggregatorTwoOrdersNonAtomic() (gas: -14825 (-0.261%))
testExecuteThroughAggregatorSingleOrder() (gas: -14825 (-0.269%))
testBuyFromLooksRareAggregatorTwoNFTsEachMarketplaceAtomic() (gas: -22238 (-0.281%))
testBuyFromLooksRareAggregatorTwoNFTsEachMarketplaceNonAtomic() (gas: -22242 (-0.282%))
testBuyFromLooksRareAggregatorAtomic() (gas: -22242 (-0.296%))
testBuyFromLooksRareAggregatorNonAtomic() (gas: -22242 (-0.298%))
testExecuteThroughAggregatorTwoOrdersNonAtomic() (gas: -14833 (-0.306%))
testExecuteThroughAggregatorTwoOrdersAtomic() (gas: -14833 (-0.306%))
testExecuteZeroOriginator() (gas: -14833 (-0.318%))
testExecuteThroughAggregatorSingleOrder() (gas: -14833 (-0.319%))
testExecuteThroughV0AggregatorTwoOrders() (gas: -14820 (-0.323%))
testExecuteThroughV0AggregatorSingleOrder() (gas: -14824 (-0.336%))
testExecuteThroughV0AggregatorTwoOrders() (gas: -14831 (-0.398%))
testExecuteThroughV0AggregatorSingleOrder() (gas: -14831 (-0.420%))
testRescueETHInsufficientAmount() (gas: -101 (-0.852%))
testRescueETHInsufficientAmount() (gas: -101 (-0.853%))
Overall gas change: -268190 (-8.178%)

use calldata in contracts/TokenReceiver.sol

For the function onERC1155Received and onERC1155BatchReceived you can use calldata instead of memory to save gas.

 abstract contract TokenReceiver {
     function onERC721Received(
         address,
@@ -16,7 +16,7 @@ abstract contract TokenReceiver {
         address,
         uint256,
         uint256,
-        bytes memory
+        bytes calldata
     ) external virtual returns (bytes4) {
         return this.onERC1155Received.selector;
     }
@@ -24,9 +24,9 @@ abstract contract TokenReceiver {
     function onERC1155BatchReceived(
         address,
         address,
-        uint256[] memory,
-        uint256[] memory,
-        bytes memory
+        uint256[] calldata,
+        uint256[] calldata,
+        bytes calldata
     ) external virtual returns (bytes4) {
         return this.onERC1155BatchReceived.selector;
     }

Savings (forge snapshot --diff):

testRescueERC1155() (gas: -326 (-0.025%))
testRescueERC1155NotOwner() (gas: -326 (-0.025%))
testExecuteAtomic() (gas: -326 (-0.130%))
testExecuteNonAtomic() (gas: -326 (-0.130%))
testBuyFromLooksRareAggregatorTwoNFTsEachMarketplaceAtomic() (gas: -83736 (-1.060%))
testBuyFromLooksRareAggregatorTwoNFTsEachMarketplaceNonAtomic() (gas: -83736 (-1.065%))
testBuyFromLooksRareAggregatorAtomic() (gas: -83736 (-1.117%))
testBuyFromLooksRareAggregatorNonAtomic() (gas: -83736 (-1.123%))
testExecuteReentrancy() (gas: -83736 (-1.189%))
testExecuteThroughAggregatorTwoOrdersAtomic() (gas: -83736 (-1.471%))
testExecuteThroughAggregatorTwoOrdersNonAtomic() (gas: -83736 (-1.477%))
testExecuteThroughAggregatorSingleOrder() (gas: -83736 (-1.521%))
testExecuteThroughAggregatorTwoOrdersNonAtomic() (gas: -83794 (-1.735%))
testExecuteThroughAggregatorTwoOrdersAtomic() (gas: -83794 (-1.735%))
testExecuteThroughV0AggregatorTwoOrders() (gas: -80725 (-1.765%))
testExecuteZeroOriginator() (gas: -83794 (-1.805%))
testExecuteThroughAggregatorSingleOrder() (gas: -83794 (-1.809%))
testExecuteThroughV0AggregatorSingleOrder() (gas: -80725 (-1.838%))
testExecuteThroughV0AggregatorTwoOrders() (gas: -80725 (-2.176%))
testExecuteThroughV0AggregatorSingleOrder() (gas: -80725 (-2.296%))
Overall gas change: -1329268 (-25.492%)

change validation check in contracts/SignatureChecker.sol

In _verify (Line 72) you can change the if condition to return early to save some gas.

         if (signer.code.length == 0) {
-            if (_recoverEOASigner(hash, signature) != signer) revert InvalidSignatureEOA();
+            if (_recoverEOASigner(hash, signature) == signer) return;
+            revert InvalidSignatureEOA();
         } else {
-            if (IERC1271(signer).isValidSignature(hash, signature) != 0x1626ba7e) revert InvalidSignatureERC1271();
+            if (IERC1271(signer).isValidSignature(hash, signature) == 0x1626ba7e) return;
+            revert InvalidSignatureERC1271();
        }

Savings (forge snapshot --diff):

testCannotSignIfWrongSignatureERC1271() (gas: -8 (-0.002%))
testSignERC1271() (gas: -10 (-0.003%))
testSignEOA() (gas: -1 (-0.003%))
testCannotSignIfWrongSignatureEOA() (gas: 1 (0.004%))
Overall gas change: -18 (-0.004%)

change function as payable in contracts/OwnableTwoSteps.sol

As all the functions are not for endusers and only handled by admins it makes sense to add payable to save gas.

-    function cancelOwnershipTransfer() external onlyOwner {
+    function cancelOwnershipTransfer() external payable onlyOwner {

-    function confirmOwnershipRenouncement() external onlyOwner {
+    function confirmOwnershipRenouncement() external payable onlyOwner {

-    function confirmOwnershipTransfer() external {
+    function confirmOwnershipTransfer() external payable {

-    function initiateOwnershipTransfer(address newPotentialOwner) external onlyOwner {
+    function initiateOwnershipTransfer(address newPotentialOwner) external payable onlyOwner {

-    function initiateOwnershipRenouncement() external onlyOwner {
+    function initiateOwnershipRenouncement() external payable onlyOwner {

Savings (forge snapshot --diff):

testRenounceOwnership() (gas: -39 (-0.063%))
testCannotConfirmRenouncementOwnershipPriorToTimelock() (gas: -48 (-0.066%))
testTransferOwnershipToNewOwner() (gas: -38 (-0.068%))
testCancelTransferOwnership() (gas: -77 (-0.076%))
testWrongRecipientCannotClaim() (gas: -48 (-0.076%))
testCannotRenounceOrInitiateTransferASecondtime() (gas: -134 (-0.153%))
testCannotCancelTransferIfNoTransferInProgress() (gas: -24 (-0.154%))
testExecuteOrdersLengthMismatch() (gas: -166 (-0.234%))
testCannotConfirmOwnershipOrRenouncementTransferIfNotInProgress() (gas: -48 (-0.255%))
testExecuteZeroOrders() (gas: -165 (-0.323%))
testExecuteReentrancy() (gas: -26052 (-0.375%))
testOwnableFunctionsOnlyCallableByOwner(address) (gas: -96 (-0.397%))
testExecuteThroughAggregatorTwoOrdersAtomic() (gas: -26051 (-0.464%))
testExecuteThroughAggregatorTwoOrdersNonAtomic() (gas: -26051 (-0.466%))
testExecuteThroughAggregatorSingleOrder() (gas: -26051 (-0.480%))
testBuyFromLooksRareAggregatorTwoNFTsEachMarketplaceAtomic() (gas: -39062 (-0.500%))
testBuyFromLooksRareAggregatorTwoNFTsEachMarketplaceNonAtomic() (gas: -39071 (-0.502%))
testBuyFromLooksRareAggregatorAtomic() (gas: -39071 (-0.527%))
testBuyFromLooksRareAggregatorNonAtomic() (gas: -39071 (-0.530%))
testExecuteThroughAggregatorTwoOrdersNonAtomic() (gas: -26050 (-0.549%))
testExecuteThroughAggregatorTwoOrdersAtomic() (gas: -26050 (-0.549%))
testExecuteZeroOriginator() (gas: -26050 (-0.571%))
testExecuteThroughAggregatorSingleOrder() (gas: -26050 (-0.573%))
testExecuteThroughV0AggregatorTwoOrders() (gas: -26040 (-0.579%))
testExecuteThroughV0AggregatorSingleOrder() (gas: -26049 (-0.604%))
testExecuteThroughV0AggregatorTwoOrders() (gas: -26047 (-0.718%))
testExecuteThroughV0AggregatorSingleOrder() (gas: -26047 (-0.758%))
Overall gas change: -469746 (-10.612%)

change onlyOwner functions to payable in contracts/LooksRareAggregator.sol

As the onlyOwner functions are not for endusers and only handled by admins it makes sense to add payable to save gas.

-    function setERC20EnabledLooksRareAggregator(address _erc20EnabledLooksRareAggregator) external onlyOwner {
+    function setERC20EnabledLooksRareAggregator(address _erc20EnabledLooksRareAggregator) external payable onlyOwner {

-    function addFunction(address proxy, bytes4 selector) external onlyOwner {
+    function addFunction(address proxy, bytes4 selector) external payable onlyOwner {

-    function removeFunction(address proxy, bytes4 selector) external onlyOwner {
+    function removeFunction(address proxy, bytes4 selector) external payable onlyOwner {

    function setFee(
        address proxy,
        uint256 bp,
        address recipient
-    ) external onlyOwner {
+    ) external payable onlyOwner {

    function approve(
        address marketplace,
        address currency,
        uint256 amount
-    ) external onlyOwner {
+    ) external payable onlyOwner {

    function rescueERC721(
        address collection,
        address to,
        uint256 tokenId
-    ) external onlyOwner {
+    ) external payable onlyOwner {

    function rescueERC1155(
        address collection,
        address to,
        uint256[] calldata tokenIds,
        uint256[] calldata amounts
-    ) external onlyOwner {
+    ) external payable onlyOwner {

Savings (forge snapshot --diff):

testRescueERC1155() (gas: -24 (-0.002%))
testRescueERC1155NotOwner() (gas: -24 (-0.002%))
testRescueERC721() (gas: -24 (-0.002%))
testRescueERC721NotOwner() (gas: -24 (-0.002%))
testExecuteWithFeesAtomic() (gas: -24 (-0.003%))
testApprove() (gas: -24 (-0.003%))
testApproveNotOwner() (gas: -24 (-0.003%))
testExecuteWithFeesNonAtomic() (gas: -24 (-0.003%))
testSetERC20EnabledLooksRareAggregator() (gas: -24 (-0.003%))
testSetERC20EnabledLooksRareAggregatorNotOwner() (gas: -24 (-0.003%))
testExecuteWithFeesAtomic() (gas: -24 (-0.004%))
testExecuteWithFeesNonAtomic() (gas: -24 (-0.004%))
testSetERC20EnabledLooksRareAggregatorAlreadySet() (gas: -48 (-0.006%))
testExecuteMaxFeeBpViolationAtomic() (gas: -48 (-0.010%))
testExecuteMaxFeeBpViolationNonAtomic() (gas: -48 (-0.011%))
testSetFee() (gas: -24 (-0.042%))
testAddFunction() (gas: -24 (-0.056%))
testRemoveFunction() (gas: -38 (-0.111%))
testRemoveFunctionNotOwner() (gas: -48 (-0.123%))
testSetFeeNotOwner() (gas: -24 (-0.171%))
testAddFunctionNotOwner() (gas: -24 (-0.176%))
testSetFeeTooHigh() (gas: -24 (-0.177%))
testBuyFromLooksRareAggregatorTwoNFTsEachMarketplaceAtomic() (gas: -18279 (-0.235%))
testBuyFromLooksRareAggregatorTwoNFTsEachMarketplaceNonAtomic() (gas: -18279 (-0.236%))
testBuyFromLooksRareAggregatorAtomic() (gas: -18279 (-0.248%))
testBuyFromLooksRareAggregatorNonAtomic() (gas: -18279 (-0.249%))
testExecuteReentrancy() (gas: -18255 (-0.263%))
testExecuteThroughAggregatorTwoOrdersAtomic() (gas: -18255 (-0.327%))
testExecuteThroughAggregatorTwoOrdersNonAtomic() (gas: -18255 (-0.328%))
testExecuteThroughAggregatorSingleOrder() (gas: -18255 (-0.338%))
testExecuteThroughAggregatorTwoOrdersNonAtomic() (gas: -18269 (-0.387%))
testExecuteThroughAggregatorTwoOrdersAtomic() (gas: -18269 (-0.387%))
testExecuteZeroOriginator() (gas: -18269 (-0.403%))
testExecuteThroughAggregatorSingleOrder() (gas: -18269 (-0.404%))
Overall gas change: -219850 (-4.724%)

Remove unused function in contracts/lowLevelCallers/LowLevelETH.sol

To save deployment size and also gas on the function ordering the function _returnETHIfAnyWithOneWeiLeft could be removed. If you want to keep it, think about changing the name so it is ordered after the other functions.

contracts/lowLevelCallers/LowLevelETH.sol
-    /**
-     * @notice Return ETH to the original sender if any is left in the payable call but leave 1 wei of ETH in the contract.
-     */
-    function _returnETHIfAnyWithOneWeiLeft() internal {
-        assembly {
-            if gt(selfbalance(), 1) {
-                let status := call(gas(), caller(), sub(selfbalance(), 1), 0, 0, 0, 0)
-            }
-        }
-    }

test/foundry/LowLevelETH.t.sol
-    function transferETHAndReturnFundsExceptOneWei() external payable {
-        _returnETHIfAnyWithOneWeiLeft();
-    }

-    function testTransferETHAndReturnFundsExceptOneWei(uint112 amount) external payable asPrankedUser(_sender) {
-        vm.deal(_sender, amount);
-        lowLevelETH.transferETHAndReturnFundsExceptOneWei{value: amount}();
-
-        if (amount > 1) {
-            assertEq(_sender.balance, amount - 1);
-            assertEq(address(lowLevelETH).balance, 1);
-        } else {
-            assertEq(_sender.balance, 0);
-            assertEq(address(lowLevelETH).balance, amount);
-        }
-    }

Savings (forge snapshot --diff):

testTransferETHAndReturnFundsToSpecificAddress(uint112) (gas: -22 (-0.039%))
testTransferETH(address,uint112) (gas: -22 (-0.041%))
Overall gas change: -44 (-0.081%)

Remove unused function in contracts/lowLevelCallers/LowLevelERC1155Transfer.sol

To save deployment size the function _returnETHIfAnyWithOneWeiLeft could be removed.