LooksRare Aggregator contest - Decurity's results

Lines of code

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

Vulnerability details

Impact

The attacker can steal the ETH of users that wanted to buy an NFT by sandwiching the victim transaction.

Proof of Concept

In non-atomic some of the orders could fail and funds should return and the end of execute() function - https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/LooksRareAggregator.sol#L108-L109. But the _returnETHIfAny() function is not checking the returned status of the call - https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/lowLevelCallers/LowLevelETH.sol#L43. If the user will buy an NFT via a contract that does not have a payable receive function then the user will lose his funds.

To perform, this attack malicious user need to buy nft before the victim. Front running such kind of the transaction from the victim where victim buying nft via smart contract without payable receive function with non-atomic option.

After funds are locked in the contract attacker could backrun the victim transaction and successfully take the victims funds from the contract via executing an order buying some other NFT for ETH - https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/LooksRareAggregator.sol#L109.

Recommended Mitigation Steps

To mitigate this issue consider check the returned status of the call in _returnETHIfAny() function.

Lines of code

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

Vulnerability details

Impact

Tokens and ETH can be sweeped from LooksRareAggregator

Proof of Concept

A malicious user may transfer out any tokens or ETH that are owned by LooksRareAggregator. To do so, a such user has to call execute() - https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/LooksRareAggregator.sol#L51 and make the trade. After executing of order user will receive all ETH that are left on the balance of LooksRareAggregator contract - https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/lowLevelCallers/LowLevelETH.sol#L43 If the user want to sweep ERC20 token he has to make tokenTrunsfer which is the condition to call the _returnERC20TokensIfAny() function - https://github.com/code-423n4/2022-11-looksrare/blob/f4c90ca149f4aeeac125605a56166297b717201a/contracts/LooksRareAggregator.sol#L241. Actually, the user can call non-atomic swap and not buy any NFT at all.

Recommended Mitigation Steps

To mitigate this issue consider saving the number of funds that were on the LooksRareAggregator contract before and return only the difference between that number and the balance at the end of the execute().