code-423n4 / 2022-09-party-findings

2 stars 0 forks source link

An operator can steal an NFT after its listed #276

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/proposals/ArbitraryCallsProposal.sol#L139 https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/proposals/ArbitraryCallsProposal.sol#L189

Vulnerability details

Impact

The ArbitraryCallsProposal contract prevents calls to setApprovalForAll on NFTs the Party owns. If an NFT is listed, calls to setApprovalForAll via the ArbitraryCallsProposal contract will succeed because the Party isn’t in possession of the NFT.

Anyone can cancel a proposal as long as the cancelDelay has passed.

The ArbitraryCallsProposal contract can be used to retrieved an NFT that may be “stuck” in a marketplace by canceling the listing.

Combining these together enables this attack.

Proof of Concept

An NFT is listed on Zora via ListOnZoraProposal The operator proposes and passes a proposal that calls setApprovalForAll on his address The operator cancels the listing once the listing delay passes The operator proposes and passes an ArbitraryCallsProposal to retrieve the NFT once the Party has the NFT the operator transfers it from the Party to himself.

Recommended Mitigation Steps

Don’t allow calls to setApprovalForAll on NFTs the Party has listed

merklejerk commented 1 year ago

This vector does not work. First, the PoC suggests executing an arbitrary call proposal while the zora proposal is in progress, which is impossible. Also, it makes an incorrect assumption that Arbitrary Call proposals do no block calls to setApprovalForAll() on precious NFTs if the party is not in possesion of it at the time of execute()-- we block those calls regardless.

HardlyDifficult commented 1 year ago

Sponsor makes good points - closing as invalid.