code-423n4 / 2023-10-party-findings

6 stars 4 forks source link

Party does not prevent creation of proposals when crowdfunding is still ongoing, which allows proposals to easily succeed #482

Open c4-submissions opened 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L553-L582

Vulnerability details

Impact

Malicious contributor can start a proposal during crowdfund phase, which would easily pass because totalVotingPower is still 0. He could, for example, create a proposal to steal funds of contributors.

Proof of Concept

Looking at the PartyGovernance#propose function, we can see that there is no check that prevents creating a proposal during crowdfund phase.

For a proposal to pass, it has to reach a threshold of the governanceValues.totalVotingPower at proposal creation. governanceValues.totalVotingPower for a party is updated when a crowdfund gets finalized, so if a proposal is created before crowdfund gets finalized, totalVotingPower would still be 0, which means any proposal created will succeed.

Here is an example of how a contributor can exploit this to steal all contributions:

Tools Used

Manual Review

Recommended Mitigation Steps

Don't allow a proposal to be created when Crowdfund is still active. This can be done by making _assertActiveMember revert if totalVotingPower is 0

Assessed type

Access Control

ydspa commented 11 months ago

The finding is valid, but the impact is not right, those early proposals would not be passed, as any accept() would revert due to divide by zero exception here: https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/party/PartyGovernance.sol#L647

QA: L

c4-pre-sort commented 11 months ago

ydspa marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

ydspa marked the issue as duplicate of #547

c4-pre-sort commented 11 months ago

ydspa marked the issue as not a duplicate

c4-pre-sort commented 11 months ago

ydspa marked the issue as primary issue

c4-judge commented 10 months ago

gzeon-c4 changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

gzeon-c4 marked the issue as grade-b

c4-judge commented 10 months ago

gzeon-c4 marked the issue as grade-a