Technical Description of Critical Vulnerability in MakerDAO Governance - OpenZeppelin blog

While working on an audit for the Coinbase team, we found a critical vulnerability in the DSChief contract of the DappHub library. The instance of this contract deployed at address0x8e2a84d6ade1e7fffee039a35ef5f19f13057152 was the core component of MakerDAO’s governance system, and had over $100 million in locked assets. We responsibly disclosed the vulnerability to the MakerDAO team, who maintains both the DappHub code and the project most affected by the issue.

The timeline of events was as follows:

• April 22nd-26th: we find and internally validate the vulnerability.
• April 26th: we disclose the vulnerability to the Maker team, in presence of representatives of the Coinbase team.
• April 30th: the Maker team presents us their remediation plan.
• May 2nd: the Maker team shares their fixed contract and we assess its security.
• May 6th: the vulnerability is publicly announced by the Maker team, followed by our own public announcement.
• May 9th: we publish this technical description of the vulnerability.

Here, we present a technical report of our findings as they pertain to the MakerDAO system, noting that other projects using the DSChief contract might also be affected by the vulnerability. The issue has now been mitigated, and the last remaining ~50 users are encouraged to migrate their MKR tokens from the legacy voting contract. A detailed account of what happened behind the scenes and how the three teams collaborated can be found in Coinbase’s report.

MakerDAO’s Governance Vulnerability

When a poll is opened for a governance decision in MakerDAO, users can lock MKR tokens in the DSChief contract to then vote for their preferred proposals, represented in the system by addresses. The vulnerable contract could be exploited in a way such that a malicious actor would have been able to:

1. Remove votes from proposals of their choice.
2. Indefinitely lock other users’ MKR tokens.

The vulnerability is based on the fact that the DSChiefApprovals contract, which DSChief extends, provides two different interfaces for voting: vote(address[]) and vote(bytes32). The first function allows users to vote for a list of addresses, while the second allows voting directly for a hash, which is the way the contract internally represents sets of proposals, known in the voting jargon as slates. When a vote is cast for a certain list of addresses, its hash is computed and stored in the contract, in a process called etching. As hashing is irreversible, it is this step which will allow the contract to later recover a list of addresses from a given hash. After the etching step, the vote(bytes32) function is called internally to complete the vote.

Votes in the DSChiefApprovals contract are encoded by approvals, a mapping associating each candidate proposal to its number of votes. When a user votes for the first time, their votes are added to the approval count of their chosen proposals. When they vote again, the approvals are subtracted from the old proposals and added to the new ones. This logic is encoded in the vote(bytes32) function, which calls the subWeight and addWeight helper functions with the hash. These crucial functions are the ones actually responsible for updating the approvals, internally retrieving the proposal addresses associated with the hash.

The crux of the issue rests on the fact that users can vote, through the vote(bytes32) function, for hashes that have not yet been etched. Normally, the addWeight and subWeight functions are balanced in the sense that approvals are either new, supported by freshly locked tokens, or taken from other proposals when a vote is changed. When a vote is cast for a hash not yet etched, however, the addWeight function will find no addresses associated with the hash, and thus no approvals will be added to the system, causing an imbalance in the system that an attacker can use maliciously.

An exploit can in fact be performed as follows. First, as described, an attacker can call the vote(bytes32) function directly to vote for a slate that has not yet been etched. This will result in a “ghost” vote, setting no approvals. The attacker then waits for genuine votes for the addresses in the slate to be issued, after which they perform another vote, this time for their preferred option. When this last vote removes the approvals from the addresses in the slate previously voted for by the attacker, it will only find approvals arising from other users’ votes, thus artificially reducing support for this address set.

This attack has the added consequence that some of the genuine voters for the affected alternatives will have their funds locked in the DSChief contract. As the free function tries to remove the approvals for the supported alternative by calling subWeight, the last users to call it will find the support entirely drained by the attacker, thus failing to reduce it even further and reverting.

There are two ways in which the exploit can be accomplished:

1. First, if there are unetched slates of viable alternatives in the vote that eventually get support, the attacker can perform the exploit for free, by voting for one of these “open” slates with a certain amount of funds, which can later be recovered. The maximum amount of stolen votes and maliciously locked tokens is proportional to the attacker’s locked funds, and depends on the amount of proposals in the poll.
2. If all combinations of candidate alternatives are already etched either by regular voting or by active prevention, an attacker can still perform the exploit at a cost. The trick here is to first vote for a slate that includes a new, artificial alternative, and then vote for it from another account. The attack is completed as before, by voting for the actually preferred option. The funds locked by the auxiliary account will be lost, but the number of stolen votes and locked funds can amount to several times the cost.

The Fix

The MakerDAO team has fixed the vulnerability in an updated version of DSChief by adding a requirement that voting is only allowed for either etched or the empty slate, which is required for vote deletion in their system. We have worked closely with them to ensure that the updated version of the contract is exempt of the vulnerability. Funds have now been migrated to the new deployed version of the contract and voting in the system can be safely resumed.