Possibility to burn all ETH in Crowdfund under some circumstances #105
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
high quality report
This report is of especially high quality
resolved
Finding has been patched by sponsor (sponsor pls link to PR containing fix)
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Lines of code
https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/Crowdfund.sol#L147
Vulnerability details
Impact
If
opts.initialContributor
is set toaddress(0)
(andopts.initialDelegate
is not), there are two problems:1.) If the crowdfund succeeds, the initial balance will be lost. It is still accredited to
address(0)
, but it is not retrievable.2.) If the crowdfund does not succeed, anyone can completely drain the contract by repeatedly calling
burn
withaddress(0)
. This will always succeed becauseCrowdfundNFT._burn
can be called multiple times foraddress(0)
. Every call will cause the initial balance to be burned (transferred toaddress(0)
).Issue 1 is somewhat problematic, but issue 2 is very problematic, because all funds of a crowdfund are burned and an attacker can specifically set up such a deployment (and the user would not notice anything special, after all these are parameters that the protocol accepts).
Proof Of Concept
This diff illustrates scenario 2, i.e. where a malicious deployer burns all contributions (1 ETH) of
contributor
. He loses 0.25ETH for the attack, but this could be reduced significantly (with moreburn(payable(address(0)))
calls:Recommended Mitigation Steps
Do not allow an initial contribution when
opts.initialContributor
is not set.The text was updated successfully, but these errors were encountered: