Incident Report: Unlock-prevention-while-voting-invariant Bug

Governance
1
Start, End [2020-12-24 1328 UTC - ongoing, estimated end Jan 2020]
Impact Specific timing of concurrent proposals could allow for malicious users to vote more than once on the same proposal
Root cause summary Incorrect time comparison

Timeline and Impact

[2020-12-24 1328 UTC] Community member Zviad submits email to security@celo.org outlining the bug
[2020-12-24 1548 UTC] Tim shared more broadly with relevant stakeholders and creates slack channel for mitigation
[2020-12-24 1652 UTC] * Nam creates incident report
* Asa and Nam assess severity of the bug
* First read is that the bug is not in need of immediate fix as current voting behaviors and trustworthiness of governance approvers significantly limits the downsides to exploiting this bug
* OpenZeppelin notified of a bug that we want to release
2020-12-24 1711 UTC * It was determined that a simple mitigation is to suggest to governance approvers not to allow concurrent proposals to have partially overlapping referendum phases with the overlap being > unlock period for Locked CELO
* “Proper” mitigation will look as follows:
   1. Fix will be created in the upcoming days, submitted on a private branch to OpenZeppelin
   2. OpenZeppelin to audit as part of release 2
   3. Fix gets merged on the public release branch
   4. Release timing will remain as planned, with proposal to upgrade on Mainnet around January 25
2020-12-26 1709 UTC * Martin shared unit test case and fix
2020-12-29 1041 UTC * Incident Report shared with OpenZeppelin
2020-12-31 1445 UTC * OpenZeppelin confirms fix
2020-01-04 2015 UTC * Martin submitted public PR to be released in Celo Core Contract Release 2

Root Causes

  • To avoid a user voting, unlocking, transferring, locking and voting again on the same proposal, the Governance contract exposes an isVoting function that the LockedGold contract uses to only allow an unlock when the account is no longer voting on a governance proposal
  • Governance keeps track of that in the Voter struct with the field mostRecentReferendumProposal
  • However, in the vote function, Governance compares the timestamp of the proposal against this value, which will always be true, and thus the most recently voted proposal will be stored (instead of the proposal with the latest referendum end time)
  • Thus, a malicious voter with a specific timing of concurrent governance proposals could (copied from original report)
    • assume ReferendumStage duration is 1 week.
      • assume there are two proposals in referendum stage: Prop1, and Prop2
      • Prop1 has 1 day left, Prop2 has 7 days left.
      • User can:
    • ** Vote for Prop2 (mostRecentReferendumProposal will become Prop2)
    • ** Vote for Prop1 (mostRecentReferendumProposal will incorrectly become Prop1)
    • ** Wait a day so Prop1 enters ExecutionStage
    • ** Unlock all CELO (since isVoting check will incorrect return false now)
    • ** Wait 3 days
    • ** Transfer CELO to a fresh new address, lock it, and vote for Prop2 since it is still in the referendum stage.
  • Mitigation is to ensure that approval is made only to proposals whose referendum phase is either non-overlapping or end time more than 3 days apart. Mitigation is subject to actions of a quorum of governance approvers.

Reflection

Previous similar incidents

None

Things that went well

  • Community disclosure process
  • Support of governance approvers meant that no exploit was possible between disclosure and fix
  • Determined that the risk was low quickly
  • Prepared a unit test and patch quickly
  • Open channel to OpenZeppelin allowed rapid audit
  • Regular, automated release process meant it was low overhead to include fix in subsequent Core Contracts Release 2

Things that didn’t go well

  • Did not have a test case covering this specific case
  • Initial PR reviews and audit did not catch this issue

Actions [Proposed]

Issues Created/Suggested

  • Examine unit test coverage of governance contract more closely
  • Allow verification tooling to apply to private branches in case a hotfix is needed

Run book Changes

None

Tactical Tensions

None

Governance Tensions

None