Core Contracts

Summary

The execute function marks a proposal as executed before verifying the success of the Timelock operation. If _timelock.executeBatch reverts, the proposal's executed flag is still set to true, leaving the proposal in an incorrect state.

Vulnerability Details

Initial State

• A governance proposal has passed voting

• The proposal is queued in the TimelockController

• The proposal aims to update critical protocol parameters (e.g., MAX_WEEKLY_EMISSION in RAACGauge)

The Issue Unfolds.

The proposal reaches execution stage and someone calls execute(proposalId)

1. The vulnerability manifests when:

   • _timelock.executeBatch() fails (e.g., due to invalid parameters)

   • The transaction reverts

   • But proposal.executed = true is already set

   • The ProposalExecuted event is emitted falsely

2. The state becomes corrupted because:

   • The proposal is marked as executed in _proposals[proposalId]

   • Future execution attempts revert with ProposalAlreadyExecuted

   • The actual governance action never took effect

If a proposal to update FeeCollector.sol's fee distribution parameters fails during execution, the protocol continues with old parameters while governance records show the update as successful.

This mirrors the MakerDAO Governance Attack (2020) where a governance delay contract incorrectly marked proposals as executed. The attack could have allowed malicious proposals to be executed multiple times or prevent legitimate proposals from being executed at all.

Impact

Proposals may be marked as executed even if their actions failed, leading to governance actions not being performed despite being recorded as successful.

The _executeProposal() function violates a core principle of atomic state updates. The critical sequence:

Updates the state before confirming the execution's success. This creates a state-execution mismatch where the governance system's recorded state doesn't reflect reality.

Tools Used

vs

Recommendations

Modify the _executeProposal function to mark executed only after confirming the Timelock operation's success. Using try/catch (preferred approach).

Using return value check.

This aligns with:

• The protocol's existing error handling patterns

• TimelockController's security model

• The checks-effects-interactions pattern