code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-looksrare/blob/main/contracts/LooksRareAggregator.sol#L108

Vulnerability details

Impact

if some tokens is sent (erroneously or not) to the contract, anyone that calls correctly LooksRareAggregator.execute will be able to steal those coins.

to execute the function using ethers as payment, these conditions must be true:

there is at least a trade (tradeData), tradeDataLength > 0
there is at least a token transfer, tokenTransfers > 0
recipient != address 0
originator is the address of the user

also the call must have been started by calling ERC20EnabledLooksRareAggregator execute function.

assuming the user can successfully execute an order through a proxy, at the end the function returns back the leftovers:

if (tokenTransfersLength > 0) _returnERC20TokensIfAny(tokenTransfers, originator);

if some tokens are left (erroneously or not) in the balance of the contract, this function will send them to the originator address, set as msg.sender, the EOA by ERC20EnabledLooksRareAggregator.execute() function.

Proof of Concept

Prerequisite is that the LooksRareAggregator contract has received somehow some ERC20 tokens.

call execute() function adding any tokenTransfer; assuming the call through proxy does not revert, the function will transfer the whole available balance to the address set as msg.sender.

Tools Used

VS Code

Recommended Mitigation Steps

get contract's balances before and after the delegatecall, then finally reimburse up to any balances before the call.

Picodes commented 1 year ago

Exactly the same as 224, please do not artificially inflate the number of issues

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Overinflated severity

code-423n4 / 2022-11-looksrare-findings