Nouns DAO contest - zkhorse's results

Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV2.sol#L903

Vulnerability details

In certain dynamic quorum configurations, a minority voting block may abuse the marginal increase in the dynamic quorum threshold to effectively veto proposals with majority support. By voting “no” in the final block before a proposal closes, the minority voters can raise the quorum requirement and defeat the proposal by causing it to fail the quorum call, even though it has overall majority support.

A group of “no” voters acting rationally should always wait to vote until the final block before a proposal closes if their combined voting power is sufficient to increase the dynamic quorum. This means that a proposal that appears uncontroversial with unanimous support may be suddenly defeated by a coordinated minority at the last minute.

When and whether this attack is possible depends on the dynamic quorum configuration. Higher quorum coefficients will make it easier for a block of minority voters to pull off this attack.

Scenario:

1. The total supply of Nouns is 100. The dynamic quorum parameters are set to a 5-20% range, with a coefficient of 1.5. This means minimum quorum is 5 votes and max is 20 votes.
2. Alice and Bob are “yes” voters, controlling 5 votes total. (2 for Alice, 3 for Bob).
3. Alice creates a proposal, and both Alice and Bob vote in support.
4. With 5 votes in favor, their proposal meets the quorum requirement and has unanimous support.
5. Eve, Mallory, and Oscar are “no” voters who would like to defeat Alice’s proposal. They each hold 1 Noun. Together, they control 3 total votes.
6. Mallory and Oscar delegate their voting power to Eve.
7. In the final block before the proposal closes, Eve casts her “no” vote.
8. The quorum requirement is raised to 9 votes. Since only 8 votes have been cast (5 yes, 3 no), the proposal fails to meet quorum and is defeated.

Impact:

Minority voting blocks may use dynamic quorum strategically to veto proposals with overall majority support. All voters will be incentivized to vote in the last block before a proposal closes.

Recommendation:

It is difficult to defend against this attack without further modifying the rules of Governor Bravo, and you may wish to accept this power asymmetry as part of “the rules of the game.” However, consider the following recommendations:

• Simulate this attack under real world conditions based on recent proposals to select dynamic quorum parameters that minimize possibility of this attack.
• Select a sufficiently low value for the dynamic quorum coefficient.
• Consider extending the proposal voting period any time a new “no” vote increases the quorum threshold. (Perhaps by a decreasing amount to ensure it cannot be perpetually extended). This gives other voters additional time to react and meet quorum when the requirement increases.

Test case:

Put the following test file in test/governance/NounsDAO/V2/dynamicQuorumVeto.test.ts:

import chai from 'chai';
import { solidity } from 'ethereum-waffle';
import { BigNumber as EthersBN } from 'ethers';
import { parseUnits } from 'ethers/lib/utils';
import hardhat from 'hardhat';

import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';

import {
    NounsDAOLogicV2, NounsDescriptorV2__factory as NounsDescriptorV2Factory, NounsToken
} from '../../../../typechain';
import {
    advanceBlocks, blockNumber, deployGovernorV2WithV2Proxy, deployNounsToken, getSigners,
    mineBlock, populateDescriptorV2, propose, setNextBlockTimestamp, setTotalSupply, TestSigners
} from '../../../utils';

const { ethers } = hardhat;

chai.use(solidity);
const { expect } = chai;

let snapshotId: number;

let token: NounsToken;
let deployer: SignerWithAddress;
let account0: SignerWithAddress;
let account1: SignerWithAddress;
let account2: SignerWithAddress;
let account3: SignerWithAddress;
let account4: SignerWithAddress;

let gov: NounsDAOLogicV2;
let proposalId: EthersBN;

async function reset() {
  if (snapshotId) {
    await ethers.provider.send("evm_revert", [snapshotId]);
    snapshotId = await ethers.provider.send("evm_snapshot", []);
    return;
  }
  token = await deployNounsToken(deployer);

  await populateDescriptorV2(
    NounsDescriptorV2Factory.connect(await token.descriptor(), deployer)
  );

  await setTotalSupply(token, 100);

  gov = await deployGovernorV2WithV2Proxy(deployer, token.address);
  snapshotId = await ethers.provider.send("evm_snapshot", []);
}

describe("NounsDAOV2#castVote/2", () => {
  beforeEach(async () => {
    [deployer, account0, account1, account2, account3, account4] =
      await ethers.getSigners();
  });

  describe("Dynamic quorum veto", () => {
    it("quorum attack", async () => {
      await reset();

      // Total supply is 100 Nouns.
      // Set the dynamic quorum to 5%-20% range,
      // with a coefficient of 1.5
      const quorumCoefficient = parseUnits("1.5", 6);
      await gov._setDynamicQuorumParams(500, 2000, quorumCoefficient);

      // Account 0 and 1 are "yes" voters, with 5 total votes.
      // Note that they hold a majority and meet quorum.
      await token.transferFrom(deployer.address, account0.address, 0);
      await token.transferFrom(deployer.address, account0.address, 1);

      await token.transferFrom(deployer.address, account1.address, 2);
      await token.transferFrom(deployer.address, account1.address, 3);
      await token.transferFrom(deployer.address, account1.address, 4);

      // Accounts 2, 3, and 4 are "no voters", with 3 total votes.
      // Note that they are a minority.
      await token.transferFrom(deployer.address, account2.address, 5);
      await token.transferFrom(deployer.address, account3.address, 6);
      await token.transferFrom(deployer.address, account4.address, 7);

      // Accounts 2 and 3 delegate to account 4
      await token.connect(account2).delegate(account4.address);
      await token.connect(account3).delegate(account4.address);

      proposalId = await propose(gov, account0);

      await mineBlock();

      // Quorum is 5 votes by default.
      let quorumVotes = await gov.quorumVotes(proposalId);
      expect(quorumVotes).to.equal(5);

      // Additional "yes" votes do not change quorum.
      await gov.connect(account0).castVote(proposalId, 1);
      quorumVotes = await gov.quorumVotes(proposalId);
      expect(quorumVotes).to.equal(5);
      await gov.connect(account1).castVote(proposalId, 1);
      quorumVotes = await gov.quorumVotes(proposalId);
      expect(quorumVotes).to.equal(5);

      // This proposal has met quorum and has unanimous support.
      // Looks like this uncontroversial proposal is gonna sail
      // through with no problem...
      const forVotes = (await gov.proposals(proposalId)).forVotes;
      expect(forVotes).to.equal(5);

      // Advance to the last block before voting ends
      const currentBlock = await blockNumber();
      const proposal = await gov.proposals(proposalId);
      await advanceBlocks(proposal.endBlock.toNumber() - currentBlock - 1);

      // The proposal is still active
      let state = await gov.state(proposalId);
      expect(state).to.equal(1);

      // The "no" block now casts their 3 votes...
      await gov.connect(account4).castVote(proposalId, 0);

      // The quorum requirement is dynamically raised.
      // Quorum is now 9 total votes.
      quorumVotes = await gov.quorumVotes(proposalId);
      expect(quorumVotes).to.equal(9);

      // The proposal failed to meet quorum and is defeated.
      // 8 total votes were cast: 5 yes, and 3 no.
      // The minority "no" voting block has effectively vetoed
      // a proposal with majority support. Rational "no" voters
      // should wait to vote until the last block before a proposal
      // closes in order to force a last minute quorum call.
      await advanceBlocks(1);
      state = await gov.state(proposalId);
      expect(state).to.equal(3);
    });
  });
});