Core Contracts

Summary

The Governance contract uses a non-unique descriptionHash as the salt parameter when scheduling operations in the TimelockController. This allows attackers to create multiple proposals with identical operation IDs, enabling denial-of-service (DoS) attacks where malicious actors can block legitimate proposals from execution or disrupt governance processes indefinitely.

Vulnerability Details

Affected Code:
In Governance.sol, the _queueProposal function generates the salt for Timelock operations using only the proposal’s descriptionHash:

Root Cause:
The salt is not bound to the proposal’s unique identifier (proposalId). Attackers can exploit this by:

1. Creating multiple proposals with identical targets, values, calldatas, and colliding descriptionHash.

2. Triggering a single operation ID in the Timelock for all such proposals.

Example Attack:

1. Proposal Creation:

   • User A submits Proposal 1 with parameters (targets, values, calldatas, descriptionA).

   • User B submits Proposal 2 with the same (targets, values, calldatas) and a crafted descriptionB such that keccak256(descriptionB) = keccak256(descriptionA).

2. Operation ID Collision:

   • Both proposals generate the same Timelock operation ID because salt = descriptionHash.

3. Blocking Legitimate Execution:

   • User B executes Proposal 2, queuing the operation in the Timelock.

   • User A attempts to execute Proposal 1 but fails due to the isOperationPending check.

   • User B cancels the operation via TimelockController.cancel(), permanently blocking Proposal 1.

4. Repeatable DoS:

   • User B repeats this process for every new proposal from User A, forcing continuous re-submissions.

Impact

• Critical Severity:

  • Governance Paralysis: Legitimate proposals can be permanently blocked.

  • Funds at Risk: If proposals include time-sensitive transactions (e.g., security patches), delays could lead to exploits.

  • Gas Griefing: Attackers waste victims’ gas by forcing proposal resubmissions.

Tools Used

• Manual code review

Recommendations

1. Bind Salt to Proposal ID:
   Modify the salt to include the proposalId, ensuring uniqueness:

   bytes32 salt = keccak256(abi.encode(proposalId, proposal.descriptionHash));

2. Track Proposal Hashes:
   Add a mapping to reject duplicate proposals:

   mapping(bytes32 => bool) public existingProposals;

   ​

   function propose(...) {

   bytes32 proposalHash = keccak256(abi.encode(targets, values, calldatas, description));

   require(!existingProposals[proposalHash], "Duplicate proposal");

   existingProposals[proposalHash] = true;

   }

3. Fail-Early in Timelock:
   In TimelockController.scheduleBatch(), revert if salt is reused:

   if (_operations[id].timestamp != 0) revert("Salt collision");

By implementing these fixes, the system will ensure unique operation IDs and prevent hash collision-based attacks.