Openzeppelin - Tests of security bugs in Governor contract

Governor.security.test.js

const { constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
const { expect } = require('chai');

const Enums = require('../helpers/enums');
const { GovernorHelper } = require('../helpers/governance');
const { clockFromReceipt } = require('../helpers/time');
const { expectRevertCustomError } = require('../helpers/customError');

const Governor = artifacts.require('$GovernorMock');
const CallReceiver = artifacts.require('CallReceiverMock');
const helperAccount = require('../helpers/account');


contract('Governor Hacks', function (accounts) {
  const [owner, proposer, voter1, voter2, voter3, voter4, executor] = accounts;

  const mode = 'blocknumber';
  const Token = artifacts.require('$ERC20Votes');

  const name = 'OZ-Governor';
  const version = '1';
  const tokenName = 'MockToken';
  const tokenSymbol = 'MTKN';
  const tokenSupply = web3.utils.toWei('100');
  const votingDelay = web3.utils.toBN(4);
  const votingPeriod = web3.utils.toBN(16);
  const value = web3.utils.toWei('1');

  describe(`security test: execute a proposal, using ${Token._json.contractName}`, () => {

    beforeEach(async function () {
      this.chainId = await web3.eth.getChainId();
      try {
        this.token = await Token.new(tokenName, tokenSymbol, tokenName, version);
      } catch {
        // ERC20VotesLegacyMock has a different construction that uses version='1' by default.
        this.token = await Token.new(tokenName, tokenSymbol, tokenName);
      }
      this.mock = await Governor.new(
        name, // name
        votingDelay, // initialVotingDelay
        votingPeriod, // initialVotingPeriod
        0, // initialProposalThreshold
        this.token.address, // tokenAddress
        10, // quorumNumeratorValue
      );
      this.receiver = await CallReceiver.new();

      this.helper = new GovernorHelper(this.mock, mode);

      // remove this to tests
      // await web3.eth.sendTransaction({ from: owner, to: this.mock.address, value });

      await this.token.$_mint(owner, tokenSupply);
      await this.helper.delegate({ token: this.token, to: voter1, value: web3.utils.toWei('10') }, { from: owner });
      await this.helper.delegate({ token: this.token, to: voter2, value: web3.utils.toWei('7') }, { from: owner });
      await this.helper.delegate({ token: this.token, to: voter3, value: web3.utils.toWei('5') }, { from: owner });
      await this.helper.delegate({ token: this.token, to: voter4, value: web3.utils.toWei('2') }, { from: owner });

      this.proposal = this.helper.setProposal(
        [
          {
            target: this.receiver.address,
            data: this.receiver.contract.methods.mockFunction().encodeABI(),
            value,
          },
        ],
        '<proposal description>',
      );

      // Before
      expect(await this.mock.proposalProposer(this.proposal.id)).to.be.equal(constants.ZERO_ADDRESS);
      expect(await this.mock.hasVoted(this.proposal.id, owner)).to.be.equal(false);
      expect(await this.mock.hasVoted(this.proposal.id, voter1)).to.be.equal(false);
      expect(await this.mock.hasVoted(this.proposal.id, voter2)).to.be.equal(false);
      expect(await web3.eth.getBalance(this.receiver.address)).to.be.bignumber.equal('0');

      expect(await this.mock.proposalEta(this.proposal.id)).to.be.bignumber.equal('0');
      expect(await this.mock.proposalNeedsQueuing(this.proposal.id)).to.be.equal(false);
    
      // Run proposal
      const txPropose = await this.helper.propose({ from: proposer });

      expectEvent(txPropose, 'ProposalCreated', {
        proposalId: this.proposal.id,
        proposer,
        targets: this.proposal.targets,
        // values: this.proposal.values,
        signatures: this.proposal.signatures,
        calldatas: this.proposal.data,
        voteStart: web3.utils.toBN(await clockFromReceipt[mode](txPropose.receipt)).add(votingDelay),
        voteEnd: web3.utils
          .toBN(await clockFromReceipt[mode](txPropose.receipt))
          .add(votingDelay)
          .add(votingPeriod),
        description: this.proposal.description,
      });

      // Snapshot and Vote

      await this.helper.waitForSnapshot();

      expectEvent(
        await this.helper.vote({ support: Enums.VoteType.For, reason: 'This is nice' }, { from: voter1 }),
        'VoteCast',
        {
          voter: voter1,
          support: Enums.VoteType.For,
          reason: 'This is nice',
          weight: web3.utils.toWei('10'),
        },
      );

      expectEvent(await this.helper.vote({ support: Enums.VoteType.For }, { from: voter2 }), 'VoteCast', {
        voter: voter2,
        support: Enums.VoteType.For,
        weight: web3.utils.toWei('7'),
      });

      expectEvent(await this.helper.vote({ support: Enums.VoteType.Against }, { from: voter3 }), 'VoteCast', {
        voter: voter3,
        support: Enums.VoteType.Against,
        weight: web3.utils.toWei('5'),
      });

      expectEvent(await this.helper.vote({ support: Enums.VoteType.Abstain }, { from: voter4 }), 'VoteCast', {
        voter: voter4,
        support: Enums.VoteType.Abstain,
        weight: web3.utils.toWei('2'),
      });

      await this.helper.waitForDeadline();
    });

    it ('should revert if the governor does not have enough ETH', async function() {
        // the proposal is initialed with 0 ETH
        expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal('0');
        await expectRevertCustomError(this.helper.execute(), 'FailedInnerCall', [])

        // the executor send not enough ETH
        await expectRevertCustomError(this.helper.execute({ from: executor, value: web3.utils.toWei('0.5') }), 'FailedInnerCall', [])
    });

    it('should accept execute a proposal if the user attach ETH', async function () {
      // the proposal is initialed with 0 ETH
      expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal('0');
      let txExecute = await this.helper.execute({ from: owner, value: value });
      expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id });
 
    });

    it('User will lose ETH if the user send more ETH than the proposal value', async function () {
        const newValue = web3.utils.toWei('2');
        await this.token.$_mint(executor, newValue);

        // the proposal is initialed with 0 ETH
        expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal('0');

        // redundant check
        expect(newValue).to.be.bignumber.gt(value);

        // verify the ETH balance of the receiver before
        expect(await web3.eth.getBalance(this.receiver.address)).to.be.bignumber.equal('0');

        let txExecute = await this.helper.execute({ from: executor, value: newValue });
        expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id });

        // verify the ETH balance of the receiver after
        const receiverBalance = await web3.eth.getBalance(this.receiver.address);
        expect(receiverBalance).to.be.bignumber.lessThan(newValue);
        console.log('receiver losses ETH: ', newValue - receiverBalance);
    });

    it('Another proposal can take advantage of the redundant ETH in the governor', async function () {
        const newValue = web3.utils.toWei('2');
        await this.token.$_mint(executor, newValue);

        let txExecute = await this.helper.execute({ from: executor, value: newValue });
        expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id });

        const anotherProposalReceiver = await CallReceiver.new();

        this.proposal = this.helper.setProposal(
            [
              {
                target: anotherProposalReceiver.address,
                data: anotherProposalReceiver.contract.methods.mockFunction().encodeABI(),
                value,
              },
            ],
            '<proposal description>',
        );

        // run proposal
        await this.helper.propose({ from: proposer });
        await this.helper.waitForSnapshot();
        await this.helper.vote({ support: Enums.VoteType.For, reason: 'This is nice' }, { from: voter1 })
        await this.helper.vote({ support: Enums.VoteType.For }, { from: voter2 })
        await this.helper.vote({ support: Enums.VoteType.Against }, { from: voter3 })
        await this.helper.vote({ support: Enums.VoteType.Abstain }, { from: voter4 })
        await this.helper.waitForDeadline();

        // execute the proposal with zero ETH
        txExecute = await this.helper.execute({ from: executor, value: 0 });
        expectEvent(txExecute, 'ProposalExecuted', { proposalId: this.proposal.id });
    });

  });

})