code-423n4 / 2024-03-zksync-findings

1 stars 1 forks source link

Governance Deadlock Risk from Compromised Actors has not been fully fixed #72

Closed c4-bot-5 closed 4 months ago

c4-bot-5 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/governance/Governance.sol#L1

Vulnerability details

Proof of Concept

In the current governance setup, specific actions are designated to governance actors, including the owner and the security council. These actions encompass a range of capabilities such as cancel, execute, executeInstant, scheduleTransparent, and scheduleShadow. Notably, the governance contract's design allows for self-calling through execute and executeInstant, extending additional privileges to the owner for functions like updateSecurityCouncil and updateDelay.

Now as explained by this bug report, the implications of this setup leads to two deadlock scenarios:

  1. Owner Compromise: If the owner's security is breached, the ability to submit upgrades via scheduleTransparent or scheduleShadow is jeopardized, effectively halting future upgrades since the owner is the sole proposer.
  2. Security Council Compromise: A compromised security council can exploit the cancel function to obstruct all owner-initiated upgrades. This blockade extends to critical governance changes such as the security council's update, given the owner's inability to utilize executeInstant and the expected nonzero proposal delay.

These vulnerabilities indicate that a single compromise can disrupt the entire governance process.

Now it's been stated that unfixed issues from the past contest are OOS for this contest, but that's not the case for this report since we are proving how the fix applied by protocol does not actually mitgates the issue, this is cause protocol have applied the "fixes" , as confirmed by the commit with the current Governance.sol, but erroneously does not fix the whole thing and only removed the security council's access to cancel().

Impact

The governance framework remains susceptible to deadlock scenarios following a compromise of key actors. In this case if the owner's keys get compromised then it's automatically impossible for anyone to schedule any upgrades since they are the only ones that have access to this function.

Recommended Mitigation Steps

As previously suggested, enforce a minimum delay for upgrades, that way there is an insurance of sufficient time for response actions by stakeholders, before updateDelay() goes through.

Assessed type

Access Control

saxenism commented 5 months ago

This is by design and highlights a centralisation risk, which is known and also Out of Scope. We consider this issue invalid.

c4-sponsor commented 5 months ago

saxenism (sponsor) disputed

alex-ppg commented 4 months ago

Similarly to #11, I cannot consider this exhibit as a valid submission based on the fact that a submission in a past contest has been properly consumed by the Sponsor's team and they do not intend to perform any follow-up alleviations, considering the present behavior correct.

c4-judge commented 4 months ago

alex-ppg marked the issue as unsatisfactory: Out of scope