Identifying proposals using its ID when vetoing makes the process vulnerable to blockchain re-orgs

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/party/PartyGovernance.sol#L665

Vulnerability details

Vulnerability details

Bug Description

In the protocol, proposals are uniquely identified through the proposal ID. The proposal Id for a new proposal is set by incrementing the lastProposalId value (check here). When vetoing a proposal via veto(), all thats needed is the proposalId of the proposal that is to be vetoed.

PartyGovernance.sol#L665

    function veto(uint256 proposalId) external {

Because of this incremental ID design, this could cause a party host to veto the wrong proposal in the event a blockchain re-org occurs.

For example:

Assume the following transactions occur in separate blocks:

• Block 1: Alice, a member calls propose() to create a proposal; its proposal ID is 1.

• Block 2: Bob is a party host, doesnt like Alice's proposal, he calls veto() with proposalID = 1 to veto the proposal.

• Block 3: James, another member calls propose() separately again, which creates another propose; its proposalID is 2.

A blockchain re-org occurs; block 1 is dropped in place of block 3:
Alice's proposal is removed, James proposal stays and now has a proposalID = 1.

Bob's call to veto() in block 2 is applied on top of the re-orged blockchain:
This causes Bob to veto() on the James proposal instead of the Alice proposal as he intended to, this is because James proposal now has a proposalID of 1.

In this scenario, due to the blockchain re-org, Bob calls veto() on a different proposal than the one he wanted. This could have severe impacts as a valid proposal is wrongly vetoed.

Impact

If a blockchain re-org occurs, a valid proposal could potientially be wrongly vetoed.

Re - orgs can happen on evm alt chains and the eth mainnet, with the last re org event on mainnet happening in may 2022, last year:

https://decrypt.co/101390/ethereum-beacon-chain-blockchain-reorg

Recommended Mitigation

Consider identifying proposals with a method that is dependent on its contents. For example, users could be expected to provide the keccak256 hash of the proposal struct properties. This can serve as the proposalID instead of an incremental proposalID uint value.

This would prevent hosts from vetoing on the wrong proposal should the block with the proposal be dropped.

Assessed type

Other

[ydspa]

QA: L

[c4-pre-sort]

ydspa marked the issue as insufficient quality report

[c4-judge]

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

[c4-judge]

gzeon-c4 marked the issue as grade-b

[adeolu98]

Hi thanks for judging, i will like to impore you to review the severity of this issue because:

• in my scenario, there is no Assets at direct risk but function of the protocol can be impacted, where in the action of one entity via a function may be misdirected or action via a function may not have the intended effect as at time of action.
• hypothetical attack path with stated assumptions which is fufilled by the hypothetical event of a blockchain re-org occuring.

These above are in line with excerpts from the severity classification for medium bugs on c4 as seen in full below:

Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

Below is also a similar issue granted as medium in the past on c4:

• Identifying publications using its ID makes the protocol vulnerable to blockchain re-orgs 2023-07-lens-findings#148

[gzeon-c4]

Lens is deployed on a chain that is more susceptible to deep reorg and sponsor has acknowledged reorg issue in a previous contest.

[adeolu98]

Lens is deployed on a chain that is more susceptible to deep reorg and sponsor has acknowledged reorg issue in a previous contest.

Okay, understood. Thank you