Trapped ETH inside LooksRareAggregator cannot be rescued by owner and may be stolen by attacker

[code423n4]

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/LooksRareAggregator.sol#L220

Vulnerability details

Impact

Trapped ETH inside LooksRareAggregator cannot be rescued by owner, as the contract only implements rescueERC721 and rescueERC1155, but not an equivalent rescueETH.

As a result, an attacker can steal any trapped ETH simply by calling execute on any order and receive the total balance of the contract via the _returnETHIfAny function at the end of the execute method from LooksRareAggregator.

Proof of Concept

1. Alice mistakenly sends ETH to LooksRareAggregator
2. The contract owner cannot rescue Alice's ETH
3. Bob creates an order costing only 1 wei and allowlists his other wallet
4. Bob calls execute and successfully fills the order. The contract's balance (consisting of Alice's ETH) will be transferred to Bob

Tools Used

Foundry

Recommended Mitigation Steps

• Implement rescueETH following the other methods rescueERC721 and rescueERC1155.
• Remove the receive function from the contract

[c4-judge]

Picodes marked the issue as duplicate of #277

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 2 (Med Risk)