code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Any user can execute a trade on the aggregator to collect trapped tokens. Which should be an action only allowed by the owner. The issue is in how _returnERC20TokensIfAny gets the amount to send back by checking the balances of the contract.

Proof of Concept

We can easily prove this attack by going to SeaportProxyERC721USDCTest and dealing $1m USDC to the aggregator beforehand.

deal(USDC, address(aggregator), 1_000_000e6);

By running the test

FOUNDRY_PROFILE=local forge test --match-contract SeaportProxyERC721USDCTest -vv

we get

[FAIL. Reason: Assertion failed.] testExecuteWithFeesAtomic() (gas: 819972)
Logs:
  Error: a == b not satisfied [uint]
    Expected: 13125000000
      Actual: 1013125000000

showing that the balance of the buyer was increased by $1m

Tools Used

forge

Recommended Mitigation Steps

computing the diff between the balance before and after the trade

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #277