LooksRare Aggregator contest - rbserver's results

An NFT aggregator protocol.

General Information

Platform: Code4rena

Start Date: 08/11/2022

Pot Size: $60,500 USDC

Total HM: 6

Participants: 72

Period: 5 days

Judge: Picodes

Total Solo HM: 2

Id: 178

League: ETH

LooksRare

Findings Distribution

Researcher Performance

Rank: 20/72

Findings: 3

Award: $264.89

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
edited-by-warden
duplicate-241

Awards

151.3257 USDC - $151.33

External Links

Lines of code

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

Vulnerability details

Impact

When the buyer uses a contract for purchasing NFTs, such contract can call the LooksRareAggregator.execute function to purchase an NFT using ETH. If the sent ETH amount when calling this function is not entirely used after the NFT trade is executed, then the _returnETHIfAny(address recipient) function call will attempt to return the unused ETH amount back to the originator, which should be the buyer contract, through executing let status := call(gas(), recipient, selfbalance(), 0, 0, 0, 0). As this low-level call executes the logics in the buyer contract's receive function, if calling this receive function reverts, such as when this receive function further calls an external contract's function that is temporarily paused which is beyond the buyer's control, then this low-level call will fail silently because the _returnETHIfAny(address recipient) function does not have any code that requires the status returned by this low-level call to be true. In this case, since calling the _returnETHIfAny(address recipient) function does not revert, the unused ETH amount fails to be returned to the buyer contract and remains trapped in the LooksRareAggregator contract. As mentioned by my other finding titled "ETH amount that is trapped in LooksRareAggregator contract can be withdrawn by user who is not LooksRareAggregator's owner", it is possible that another user, who calls the LooksRareAggregator.execute function before the LooksRareAggregator contract's owner calls the rescueETH function, gains such trapped ETH amount; hence, the buyer contract can lose the unused ETH amount sent by it after such amount becomes trapped in the contract.

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

    function execute(
        TokenTransfer[] calldata tokenTransfers,
        TradeData[] calldata tradeData,
        address originator,
        address recipient,
        bool isAtomic
    ) external payable nonReentrant {
        ...
        _returnETHIfAny(originator);

        emit Sweep(originator);
    }

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

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

Proof of Concept

First, please add test\foundry\utils\MockBuyer.sol as follows.

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

contract MockBuyer {
    receive() external payable {
        // This MockBuyer contract simulates a situation where executing logics in the buyer contract's receive function reverts;
        //   for example, such logics could include an external function call that reverts
        //   because the corresponding function in the external contract is temporarily paused.
        revert();
    }
}

Then, please add the following code in test\foundry\LooksRareAggregatorTrades.t.sol.

  1. Import MockBuyer.sol as follows.
import "./utils/MockBuyer.sol";
  1. Add the following test. This test will pass to demonstrate the described scenario.
    function testETHAmountSentByBuyerContractThatIsUnusedAfterPurchasingNFTCanBeTrappedInAggregator() public {
        vm.createSelectFork(vm.rpcUrl("mainnet"), 15_282_897);

        aggregator = new LooksRareAggregator();
        vm.deal(address(aggregator), 0);

        LooksRareProxy looksRareProxy = new LooksRareProxy(LOOKSRARE_V1, address(aggregator));
        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: ""
        });

        // The buyer uses the mockBuyer contract for purchasing NFTs.
        // It simulates a situation where executing logics in the buyer contract's receive function reverts;
        //   for example, such logics could include an external function call that reverts
        //   because the corresponding function in the external contract is temporarily paused.
        MockBuyer mockBuyer = new MockBuyer();
        vm.deal(address(mockBuyer), orders[0].price + 1 ether);

        // the aggregator owns 0 ETH at this moment
        assertEq(address(aggregator).balance, 0);

        // The mockBuyer contract calls the LooksRareAggregator.execute function to purchase the NFT using ETH
        //   while sending an ETH amount that is more than the NFT's purchase price.
        vm.prank(address(mockBuyer));
        aggregator.execute{value: address(mockBuyer).balance}(tokenTransfers, tradeData, address(mockBuyer), address(mockBuyer), false);

        // Although the mockBuyer contract receives the purchased NFT after calling the LooksRareAggregator.execute function,
        //   the difference between the sent ETH amount and the NFT's purchase price is trapped in the aggregator
        //   instead of being returned to the mockBuyer contract.
        assertEq(IERC721(BAYC).ownerOf(7139), address(mockBuyer));
        assertEq(address(mockBuyer).balance, 0);
        assertEq(address(aggregator).balance, 1 ether);
    }

Tools Used

VSCode

Similar to the _transferETH function, the _returnETHIfAny(address recipient) function (https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/lowLevelCallers/LowLevelETH.sol#L43-L49) can be updated to the following code.

    function _returnETHIfAny(address recipient) internal {
        bool status = true;

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

        if (!status) revert ETHTransferFail();
    }

Although the _returnETHIfAny() and _returnETHIfAnyWithOneWeiLeft functions are not called by the contracts in scope, these functions can be updated to be similar to the _transferETH function as well. The current implementations of these functions are shown below.

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/lowLevelCallers/LowLevelETH.sol#L32-L38

    function _returnETHIfAny() internal {
        assembly {
            if gt(selfbalance(), 0) {
                let status := call(gas(), caller(), selfbalance(), 0, 0, 0, 0)
            }
        }
    }

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/lowLevelCallers/LowLevelETH.sol#L54-L60

    function _returnETHIfAnyWithOneWeiLeft() internal {
        assembly {
            if gt(selfbalance(), 1) {
                let status := call(gas(), caller(), sub(selfbalance(), 1), 0, 0, 0, 0)
            }
        }
    }

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/lowLevelCallers/LowLevelETH.sol#L19-L27

    function _transferETH(address _to, uint256 _amount) internal {
        bool status;

        assembly {
            status := call(gas(), _to, _amount, 0, 0, 0, 0)
        }

        if (!status) revert ETHTransferFail();
    }

#0 - c4-judge

2022-11-19T11:27:16Z

Picodes marked the issue as duplicate of #241

#1 - c4-judge

2022-12-16T13:58:32Z

Picodes changed the severity to 2 (Med Risk)

#2 - c4-judge

2022-12-16T13:58:33Z

Picodes marked the issue as satisfactory

Findings Information

Awards

77.2215 USDC - $77.22

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
duplicate-277

External Links

Lines of code

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/TokenRescuer.sol#L34-L38 https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/ERC20EnabledLooksRareAggregator.sol#L28-L37 https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/LooksRareAggregator.sol#L51-L112 https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/LooksRareAggregator.sol#L241-L251 https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/lowLevelCallers/LowLevelERC20Transfer.sol#L46-L57

Vulnerability details

Impact

It is possible that an ERC20 token amount is trapped in the LooksRareAggregator contract; for instance, a user can accidentally send some of an ERC20 token to this contract. Because the LooksRareAggregator contract inherits the TokenRescuer contract, the LooksRareAggregator contract's owner is able to call the rescueERC20 function to transfer such trapped ERC20 token amount to an appropriate address. However, after noticing that there is an ERC20 token amount being trapped in the LooksRareAggregator contract and the contract's owner has not yet called the rescueERC20 function for the corresponding token, a user can call the ERC20EnabledLooksRareAggregator.execute function to, for example, purchase an NFT using ETH while setting the tokenTransfers input to include 0 of the trapped ERC20 token. In this way, when the ERC20EnabledLooksRareAggregator.execute function calls the LooksRareAggregator.execute function, which executes the _returnERC20TokensIfAny and _executeERC20DirectTransfer functions for the trapped ERC20 token, this user is able to withdraw the LooksRareAggregator contract's balance of the trapped ERC20 token that includes such token's trapped amount. After this user calls the ERC20EnabledLooksRareAggregator.execute function, the LooksRareAggregator contract's owner's rescueERC20 function call would revert since the LooksRareAggregator contract's balance of the trapped ERC20 token has become 0 and executing IERC20(currency).balanceOf(address(this)) - 1 underflows. In this situation, the LooksRareAggregator contract's owner is not able to transfer the trapped ERC20 token amount, which violates the design. Moreover, the user, who calls the ERC20EnabledLooksRareAggregator.execute function in this case, gains the trapped ERC20 token amount, which is unfair to the user, who sends the trapped ERC20 token amount and loses all of it.

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

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

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

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

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

    function execute(
        TokenTransfer[] calldata tokenTransfers,
        TradeData[] calldata tradeData,
        address originator,
        address recipient,
        bool isAtomic
    ) external payable nonReentrant {
        ...

        if (tokenTransfersLength > 0) _returnERC20TokensIfAny(tokenTransfers, originator);
        ...
    }

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

    function _returnERC20TokensIfAny(TokenTransfer[] calldata tokenTransfers, address recipient) private {
        uint256 tokenTransfersLength = tokenTransfers.length;
        for (uint256 i; i < tokenTransfersLength; ) {
            uint256 balance = IERC20(tokenTransfers[i].currency).balanceOf(address(this));
            if (balance > 0) _executeERC20DirectTransfer(tokenTransfers[i].currency, recipient, balance);

            unchecked {
                ++i;
            }
        }
    }

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

    function _executeERC20DirectTransfer(
        address currency,
        address to,
        uint256 amount
    ) internal {
        (bool status, bytes memory data) = currency.call(abi.encodeWithSelector(IERC20.transfer.selector, to, amount));

        if (!status) revert ERC20TransferFail();
        if (data.length > 0) {
            if (!abi.decode(data, (bool))) revert ERC20TransferFail();
        }
    }

Proof of Concept

Please add the following code in test\foundry\LooksRareAggregatorTrades.t.sol.

  1. Import stdError and ERC20EnabledLooksRareAggregator as follows.
import {stdError} from "../../lib/forge-std/src/StdError.sol";
import {ERC20EnabledLooksRareAggregator} from "../../contracts/ERC20EnabledLooksRareAggregator.sol";
  1. Add the following test. This test will pass to demonstrate the described scenario.
    function testERC20TrappedInAggregatorCanBeWithdrawnByNonOwner() public {
        vm.createSelectFork(vm.rpcUrl("mainnet"), 15_282_897);

        aggregator = new LooksRareAggregator();
        vm.deal(address(aggregator), 0);

        LooksRareProxy looksRareProxy = new LooksRareProxy(LOOKSRARE_V1, address(aggregator));
        vm.deal(address(looksRareProxy), 0);

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

        ERC20EnabledLooksRareAggregator erc20EnabledAggregator = new ERC20EnabledLooksRareAggregator(address(aggregator));
        aggregator.setERC20EnabledLooksRareAggregator(address(erc20EnabledAggregator));

        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: ""
        });

        MockERC20 mockERC20 = new MockERC20();

        // alice owns 1 ether mockERC20 at this moment
        address alice = address(1);
        mockERC20.mint(alice, 1 ether);

        // alice accidentally sends 1 ether mockERC20 to the aggregator
        vm.prank(alice);
        mockERC20.transfer(address(aggregator), 1 ether);

        vm.deal(_buyer, orders[0].price);

        // After noticing that the aggregator at this moment owns the 1 ether mockERC20 that was accidentally sent by alice
        //   and the LooksRareAggregator.rescueERC20 function is not yet called by the aggregator's owner,
        //   _buyer calls the ERC20EnabledLooksRareAggregator.execute function to purchase the NFT using ETH
        //   while setting the tokenTransfers input to include 0 mockERC20.
        vm.startPrank(_buyer);

        TokenTransfer[] memory tokenTransfers = new TokenTransfer[](1);
        tokenTransfers[0].amount = 0;
        tokenTransfers[0].currency = address(mockERC20);

        erc20EnabledAggregator.execute{value: orders[0].price}(tokenTransfers, tradeData, _buyer, false);

        vm.stopPrank();

        // After the ERC20EnabledLooksRareAggregator.execute function is called,
        //   _buyer receives the purchased NFT and also the 1 ether mockERC20 that was accidentally sent by alice.
        assertEq(IERC721(BAYC).ownerOf(7139), _buyer);
        assertEq(mockERC20.balanceOf(_buyer), 1 ether);
        
        // Calling the LooksRareAggregator.rescueERC20 function by the aggregator's owner reverts
        //   because subtracting 1 from the aggregator's mockERC20 balance, which is 0 at this moment, underflows.
        vm.expectRevert(stdError.arithmeticError);
        aggregator.rescueERC20(address(mockERC20), alice);

        // the aggregator's owner has failed to recover the 1 ether mockERC20, which was accidentally sent, for alice
        assertEq(mockERC20.balanceOf(alice), 0);
    }

Tools Used

VSCode

At the beginning of the LooksRareAggregator.execute function, for each ERC20 token in the tokenTransfers input, IERC20(tokenTransfers[i].currency).balanceOf(address(this)) - tokenTransfers[i].amount can be executed to get the LooksRareAggregator contract's balance of the tokenTransfers[i].currency token before receiving the corresponding token amount from the originator; then, for each of these ERC20 tokens, such balance amount should be excluded when calling the _returnERC20TokensIfAny and _executeERC20DirectTransfer functions for transferring the token amount that is left back to the originator.

#0 - c4-judge

2022-11-19T11:32:27Z

Picodes marked the issue as duplicate of #277

#1 - c4-judge

2022-12-16T13:58:21Z

Picodes changed the severity to 2 (Med Risk)

#2 - c4-judge

2022-12-16T13:58:21Z

Picodes marked the issue as satisfactory

[L-01] CALLING rescueERC721 FUNCTION CAN POSSIBLY LOCK THE TRANSFERRED ERC721 TOKEN

Calling the rescueERC721 function further calls the _executeERC721TransferFrom function. Calling the _executeERC721TransferFrom function then executes (bool status, ) = collection.call(abi.encodeWithSelector(IERC721.transferFrom.selector, from, to, tokenId)). Since the IERC721.transferFrom function selector is used, if the address corresponding to the to input is a contract that does not support ERC721 protocol, then it is possible that the ERC721 token is transferred to an address that cannot interact with the corresponding ERC721 contract. As a result, the transferred ERC721 is locked and cannot be retrieved.

For reference, OpenZeppelin's documentation for transferFrom states: " Note that the caller is responsible to confirm that the recipient is capable of receiving ERC721 or else they may be permanently lost." Please consider updating the _executeERC721TransferFrom function to use the corresponding IERC721.safeTransferFrom function selector instead of the IERC721.transferFrom function selector.

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

    function rescueERC721(
        address collection,
        address to,
        uint256 tokenId
    ) external onlyOwner {
        _executeERC721TransferFrom(collection, address(this), to, tokenId);
    }

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/lowLevelCallers/LowLevelERC721Transfer.sol#L21-L29

    function _executeERC721TransferFrom(
        address collection,
        address from,
        address to,
        uint256 tokenId
    ) internal {
        (bool status, ) = collection.call(abi.encodeWithSelector(IERC721.transferFrom.selector, from, to, tokenId));
        if (!status) revert ERC721TransferFromFail();
    }

[L-02] LACK OF GOVERNANCE AND VOTING MECHANISM FOR CALLING rescueERC721, rescueERC1155, rescueETH, and rescueERC20 FUNCTIONS

The LooksRareAggregator contract's owner has the privilege to call the rescueERC721, rescueERC1155, rescueETH, and rescueERC20 functions to transfer the corresponding trapped assets to the specified addresses. Looking at https://docs.looksrare.org/developers/deployed-contract-addresses, it appears to be that this protocol does not have a governance contract that lets the community to vote. This means that the LooksRareAggregator contract's owner cannot be such governance contract currently, and there is no guarantee that the LooksRareAggregator contract's current owner will not become compromised or malicious in the future. When the LooksRareAggregator contract's owner becomes compromised or malicious, this owner can call the rescueERC721, rescueERC1155, rescueETH, and rescueERC20 contracts to immediately withdraw the corresponding trapped assets so these assets would be lost forever. Please consider setting up a governance contract and transferring the ownership of the LooksRareAggregator contract to this governance contract so the community can vote on how these rescueERC721, rescueERC1155, rescueETH, and rescueERC20 functions can be called.

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

    function rescueERC721(
        address collection,
        address to,
        uint256 tokenId
    ) external onlyOwner {
        _executeERC721TransferFrom(collection, address(this), to, tokenId);
    }

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

    function rescueERC1155(
        address collection,
        address to,
        uint256[] calldata tokenIds,
        uint256[] calldata amounts
    ) external onlyOwner {
        _executeERC1155SafeBatchTransferFrom(collection, address(this), to, tokenIds, amounts);
    }

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/TokenRescuer.sol#L22-L26

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

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

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

[L-03] MISSING address(0) CHECKS FOR CRITICAL ADDRESS INPUTS

To prevent unintended behaviors, critical address inputs should be checked against address(0).

Please consider checking _aggregator in the following constructor. https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/ERC20EnabledLooksRareAggregator.sol#L21-L23

    constructor(address _aggregator) {
        aggregator = ILooksRareAggregator(_aggregator);
    }

Please consider checking _marketplace and _aggregator in the following constructor. https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/proxies/LooksRareProxy.sol#L37-L40

    constructor(address _marketplace, address _aggregator) {
        marketplace = ILooksRareExchange(_marketplace);
        aggregator = _aggregator;
    }

Please consider checking _marketplace and _aggregator in the following constructor. https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/proxies/SeaportProxy.sol#L45-L48

    constructor(address _marketplace, address _aggregator) {
        marketplace = SeaportInterface(_marketplace);
        aggregator = _aggregator;
    }

[N-01] REPEATED AND REDUNDANT CODE CAN BE REMOVED

TokenRescuer is imported twice in LooksRareAggregator.sol. Please remove one of these import code lines.

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

import {TokenRescuer} from "./TokenRescuer.sol";
...
import {TokenRescuer} from "./TokenRescuer.sol";

[N-02] CONSTANTS CAN BE USED INSTEAD OF MAGIC NUMBERS

To improve readability and maintainability, constants can be used instead of magic numbers. Please note that the following magic numbers are not flagged in https://gist.github.com/Picodes/0554505ff357a2da3bca5fbb839ee7da.

Please consider replacing the following magic numbers, such as 2, with constants.

contracts\LooksRareAggregator.sol
  158: if (bp > 10000) revert FeeTooHigh();    

contracts\proxies\SeaportProxy.sol
  147: uint256 orderFee = (orders[i].price * feeBp) / 10000;   
  207: uint256 orderFee = (price * feeBp) / 10000; 
  246: offer[0].itemType = ItemType(uint8(order.collectionType) + 2);  

[N-03] INCOMPLETE NATSPEC COMMENT

NatSpec comments provide rich code documentation. The @param comment is missing for the following _returnETHIfAny function. Please consider completing the NatSpec comment for it.

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

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

[N-04] MISSING NATSPEC COMMENTS

NatSpec comments provide rich code documentation. NatSpec comments are missing for the following functions. Please consider adding them.

contracts\ERC20EnabledLooksRareAggregator.sol
  39: function _pullERC20Tokens(TokenTransfer[] calldata tokenTransfers, address source) private {    

contracts\LooksRareAggregator.sol
  222: function _encodeCalldataAndValidateFeeBp(   
  241: function _returnERC20TokensIfAny(TokenTransfer[] calldata tokenTransfers, address recipient) private {   

contracts\TokenTransferrer.sol
  14: function _transferTokenToRecipient( 

contracts\proxies\LooksRareProxy.sol
  107: function _executeSingleOrder(   

contracts\proxies\SeaportProxy.sol
  88: function _executeAtomicOrders(  
  136: function _handleFees(   
  166: function _transferFee(   
  178: function _executeNonAtomicOrders(   
  227: function _populateParameters(BasicOrder calldata order, OrderExtraData memory orderExtraData)   

#0 - c4-judge

2022-11-21T17:13:23Z

Picodes marked the issue as grade-b

#1 - 0xhiroshi

2022-11-24T18:26:47Z

only N-01 is valid

#2 - c4-sponsor

2022-11-24T18:26:51Z

0xhiroshi requested judge review

#3 - c4-judge

2022-12-11T13:31:22Z

Picodes marked the issue as grade-a

#4 - c4-judge

2022-12-11T22:23:00Z

Picodes marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter