c4-bot-10 commented 6 months ago

Lines of code

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

Vulnerability details

Impact

In both the GuildGovernor and GuildVetoGovernor contract the quorum can be changed between the time of a proposal creation time and its execution which may potentially impact the acceptance or refusal of certain proposals

Proof of Concept

The ability to change the quorum value between the creation and execution of proposals can lead to the following issues:

Proposal Integrity: The dynamic nature of the quorum allows for manipulation of the voting requirements, potentially impacting the integrity of the proposal acceptance process.
Unpredictable Governance: The lack of a fixed quorum for each block number introduces uncertainty in the governance process, making it difficult for participants to anticipate the voting requirements at the time of proposal execution.

The vulnerability is evident in the following code snippet:

/// @notice The minimum number of votes needed for a vote to pass.
function quorum(uint256 /* blockNumber */) public view override returns (uint256) {
    //@audit quorum can be changed between the proposals creation and execution time which may affect the proposal acceptance or refusal, should instead have a quorum for each block number
    return _quorum;
}

The absence of a specific quorum value for each block number allows the _quorum value to be modified without restrictions, potentially leading to governance inconsistencies.

Tools Used

Manual review

Recommended Mitigation Steps

To address this vulnerability and enhance the security of the governance contract, it is recommended to implement a fixed quorum for each block number. This can be achieved by maintaining a mapping of block numbers to their corresponding quorum values. The quorum function should then retrieve the appropriate quorum value based on the block number provided.

Example:

mapping(uint256 => uint256) private _blockNumberQuorums;

function setQuorum(uint256 blockNumber, uint256 newQuorum) external onlyOwner {
    _blockNumberQuorums[blockNumber] = newQuorum;
}

function quorum(uint256 blockNumber) public view override returns (uint256) {
    return _blockNumberQuorums[blockNumber];
}

This modification ensures that the quorum is fixed for each block number, providing a more secure and predictable governance process.

The governance contract allows the dynamic modification of the quorum value between the creation and execution of proposals. This flexibility in adjusting the quorum may introduce a security vulnerability, potentially impacting the acceptance or refusal of proposals. It is recommended to establish a fixed quorum for each block number to enhance the security and predictability of the governance process.