code-423n4 / 2023-12-ethereumcreditguild-findings

9 stars 5 forks source link

Unexpected quorum changes can affect ongoing votes in `GuildVetoGovernor.sol` #1268

Closed c4-bot-8 closed 5 months ago

c4-bot-8 commented 6 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/GuildVetoGovernor.sol#L53-L57

Vulnerability details

Impact

The quorum function in the GuildVetoGovernor.sol contract ignores the blockNumber parameter. This means that the quorum can be modified while a vote is ongoing. This could potentially lead to unexpected behavior, as users may have voted differently if the quorum was different at the time of their vote.

The quorum function is designed to return the minimum number of votes needed for a vote to pass. However, due to the lack of consideration for the blockNumber, the quorum can be changed during a vote, which could potentially affect the outcome of the vote.

function quorum(
    uint256 /* blockNumber*/
) public view override returns (uint256) {        
    return _quorum;
}

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/GuildVetoGovernor.sol#L53-L57

Proof of Concept

Consider a scenario where Alice and Bob are participating in a vote. Alice votes first when the quorum is set to a certain value. However, before Bob casts his vote, the quorum is changed. This change could potentially affect the outcome of the vote, as Bob may have voted differently if the quorum was the same as when Alice voted.

This issue is present in the GuildVetoGovernor.sol file.

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this issue, it is recommended to consider the blockNumber when determining the quorum. This could be achieved by storing the quorum value at the time of each vote, ensuring that it remains constant throughout the voting process. This would prevent unexpected changes to the quorum from affecting ongoing votes.

Assessed type

Governance

0xSorryNotSorry commented 5 months ago

As per the docs, the Governor actions are trusted. QA

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as insufficient quality report

Trumpero commented 5 months ago

An instance of Governor's centralization risk ([M-01] Centralization risk for privileged functions) -> OOS

c4-judge commented 5 months ago

Trumpero marked the issue as unsatisfactory: Out of scope