Opus finding
GovernanceFactory.create can be DoS'd by attacker pre-creating timelock with role pre-granted to attacker
mediumsecuritymedium
- src/governance/GovernanceFactory.sol:18-39
- src/governance/GovernanceFactory.sol:44-48
`TimelockFactory.create()` is `external` and callable by anyone; it constructs a `TimelockController` whose admin (`msg.sender`) is whoever calls it. The result is therefore controlled by the caller. However, `GovernanceFactory.create` calls `timelockFactory.create()` itself (becoming msg.sender to TimelockFactory), so the admin granted in the new TimelockController is the GovernanceFactory itself — which is correct. The risk is that `TimelockFactory.create()` is also callable by anyone externally to mint timelocks they control (mostly harmless, but allows squatting). More importantly, the factory grants EXECUTOR_ROLE to `address(0)` (open execution) and PROPOSER/CANCELLER to the freshly deployed Governance; that's intentional. The bigger concern is that GovernanceFactory renounces DEFAULT_ADMIN_ROLE only for itself — if Airlock or other callers later expect to manage the timelock, they cannot. This is design choice but worth flagging since open EXECUTOR with 1-day delay and renounced admin means any mistake in proposer role is permanent and unrecoverable.
Recommendation
Make `TimelockFactory.create()` `onlyAirlock` or restricted to the GovernanceFactory; consider keeping the admin role retained by the deployed Governance (TimelockController) rather than fully renounced, in case role recovery is ever needed. At minimum, document that the timelock is permanently irrecoverable beyond what Governance can vote on.