Timelock Hash Collision in Governance Contract

Summary

The current implementation of the Governance contract contains a critical vulnerability related to the generation of timelock operation IDs. This vulnerability arises from the use of proposal descriptions as the sole basis for generating the salt used in the timelock ID. If two proposals share the same description, they will collide, leading to significant governance issues.

Current Code Issue

The relevant code snippet is as follows:

Issue Explanation

1. Hash Collision: The salt used for generating the timelock ID is derived directly from the proposal's description hash. If two proposals have identical descriptions, they will produce the same hash value when passed through keccak256. This results in both proposals being assigned the same timelock ID.

2. Impact on Proposal Execution: When two proposals share the same ID, the second proposal (with the same description) cannot be executed because the timelock mechanism will treat it as a duplicate operation. This effectively blocks any subsequent proposals with the same description from being processed, leading to a denial of service for those proposals.

Example Attack Scenario

• Proposal A: A proposal is created with the description "Upgrade v1". It generates a unique ID (e.g., 0x123).

• Proposal B: Another proposal is created with the same description "Upgrade v1". It generates the same ID (0x123).

• Outcome: When attempting to execute Proposal B, the timelock mechanism recognizes the ID as already used (from Proposal A) and blocks the execution, preventing Proposal B from being processed.

Impact

If the timelock hash collision vulnerability remains in the Governance contract, the following consequences are possible:

1. Denial of Service (DoS) to Governance Proposals: Any governance proposals that share the exact same description will collide when trying to be queued in the TimelockController. This can prevent important proposals from being executed.

2. Governance Stalling and Censorship: A malicious actor could exploit this vulnerability by creating a "dummy" proposal with a description that they know will be reused in a future important proposal. This could effectively block critical governance actions.

3. Erosion of Trust and Participation: If token holders become aware that proposals can be blocked simply by reusing descriptions, they may lose faith in the governance process, leading to decreased participation in voting.

4. Protocol Instability and Financial Loss: Critical proposals related to bug fixes or emergency adjustments could be blocked, leading to protocol instability and potential financial losses.

Conclusion

This vulnerability is of high severity because it can lead to significant governance issues, allowing for denial of service against proposals that happen to have the same description. This could be exploited by malicious actors to block important governance actions.

Recommended Mitigation

To address this vulnerability, the timelock ID generation should incorporate additional unique identifiers beyond just the proposal description. This could include:

• A unique proposal ID or nonce.

• A timestamp or block number to ensure uniqueness.

Proposed Code Update

Here’s a simplified version of how the proposal ID could be modified to include a unique identifier:

By implementing this change, the risk of hash collisions can be mitigated, ensuring that each proposal can be uniquely identified and executed without interference from proposals with identical descriptions.