LooksRare Aggregator contest - R2's results

Lines of code

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

Vulnerability details

Impact

To calculate change (not used tokens/ETH) you are checking entire contract balance, and not checking if contract balance was not empty at transactions start

Proof of Concept

1. Alice accidentally sent 100 tokens A directly to LooksRareAggregator.
2. Bob executes trade using 10 tokens A (approve it to ERC20EnabledLooksRareAggregator).
3. In _returnERC20TokensIfAny() you are checking balance: uint256 balance = IERC20(tokenTransfers[i].currency).balanceOf(address(this)). And balance = 100 so Bob will receive 100 A tokens

Tools Used

vs code

Recommended Mitigation Steps

Check balance before sending Bob's tokens and after trade. And send to Bob only difference

Lines of code

https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/SeaportProxy.sol#L116

Vulnerability details

Impact

Before calling marketplace.fulfillAvailableAdvancedOrders() you should approve tokens using in advancedOrders But you haven't approval calls inside execute() function

You have dedicated function LooksRareAggregator.approve() but you have to call it for all tokens for unlimited count It's not safe, because marketplace may be hacked And if you hadn't call LooksRareAggregator.approve() for some token, it will be impossible to use that token for transactions

Proof of Concept

No approve()calls: https://github.com/code-423n4/2022-11-looksrare/blob/e3b2c053f722b0ca2dce3a3eb06f64859b8b7a6f/contracts/proxies/SeaportProxy.sol#L116

Tools Used

vs code

Recommended Mitigation Steps

Before calling marketplace.fulfillAvailableAdvancedOrders() call IERC20(token).approve() for each token using in advancedOrders