Anyone can skim tokens/ETH accidentally sent directly to your contract

[code423n4]

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

[c4-judge]

Picodes marked the issue as duplicate of #277

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[c4-judge]

Picodes marked the issue as satisfactory