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

0 stars 0 forks source link

QA Report #248

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

See the markdown file with the details of this report here.

c4-judge commented 1 year ago

Picodes marked the issue as grade-b

0xhiroshi commented 1 year ago

If someone accidentally sends ERC20 / ETH to the LooksRareAggregator contract, the next user using the aggregator will be given these token - invalid, it's free for all LooksRareAggregator may be used to sell NFTs / mass accept offers instead of buying them in the future. Expose a risk for a hacker to steal approved NFTs in case the governance key got compromised - invalid, there is no code to sell NFT in this contract

c4-sponsor commented 1 year ago

0xhiroshi marked the issue as sponsor disputed

Picodes commented 1 year ago

The second one is valid to me has any allowance to LooksRareAggregator or ERC20EnabledLooksRareAggregator could be exploited by a malicious owner: you just need to addFunction.

0xhiroshi commented 1 year ago

@Picodes ERC20EnabledLooksRareAggregator has no owner, and if we want to add a function to sell NFTs, we will follow a similar architecture as ERC20EnabledLooksRareAggregator, which does not grant NFT approvals to LooksRareAggregator so even if LooksRareAggregator is compromised the attacker cannot transfer NFTs out of users' wallets.

Picodes commented 1 year ago

You are right, my bad