LooksRare Aggregator contest - carrotsmuggler's results

Lines of code

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/TokenRescuer.sol#L14-L39 https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/OwnableTwoSteps.sol#L125-L127

Vulnerability details

Impact

Contract TokenRescuer inherits the contract OwnableTwoSteps but does not set any delay. There should be a constructor in TokenRescuer responsible for setting the delay as described in the comments of the OwnableTwoSteps contract. Without any delay, ownership can be changed/revoked within a single block.

Proof of Concept

contract TokenRescuer is OwnableTwoSteps, LowLevelETH, LowLevelERC20Transfer {
    error InsufficientAmount();
    
   // @audit: Missing constructor/ call to _setupDelayForRenouncingOwnership

    function rescueETH(address to) external onlyOwner
    function rescueERC20(address currency, address to) external onlyOwner
}

Tools Used

forge

Recommended Mitigation Steps

Add a constructor to TokenRescuer setting a delay of 1 day for ownership changes/revoking.

Lines of code

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

Vulnerability details

Impact

The TokenRescuer contract helps to rescue eth and erc20 tokens and can only be called by the owner. However, any trapped eth or erc20 tokens can actually be taken out by anyone. Essentially the function and access control of the TokenReceiver contract can be bypassed. This can be a low risk issue, but classified as med since it directly bypasses some access control.

Proof of Concept

1. List any bogus nft for any price
2. Use ERC20EnabledLooksRareAggregator and include the erc20 token you want to get out of the contract in the tokenTransfers array
3. After purchasing the listed nft, all erc20 tokens and ETH in the contract will be sent to the receiver as long as it was included in the tokenTransfers array

Tools Used

foundry

Recommended Mitigation Steps

Store the purchase price of the NFT and the credited amount and only return the balance difference, not the entire contract balance.

1. LooksRareAggregator fee can be set to 10000bp

LooksRareAggregator only checks if the set fee bp <= 10000. This allows instances of 10000bp fees, or 100% fees.

2. ERC20EnabledAggregator doesnt have recovery option

LooksRareAggregator has token recovery options for erc20, erc721 and erc1155. However ERC20EnabledAggregator does not have recovery options for any of them. It is unlikely ERC20EnabledAggregator will ever come across erc721 or erc1155, but it is quite likely some user might send erc20 to the contract by mistake which will become unrecoverable.

3. WETH returns for ETH purchases wont be returned

If protocol returns WETH on ETH purchases, they wont be returned to the user