[Lines of code](https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L964-L972) # Vulnerability details ## Impact The following _writeQuorumParamsCheckpoint function is used to record dynamic quorum votes parameters at a block of interest. function _writeQuorumParamsCheckpoint(DynamicQuorumParams memory params) internal { uint32 blockNumber = safe32(block.number, 'block number exceeds 32 bits'); uint256 pos = quorumParamsCheckpoints.length; if (pos > 0 && quorumParamsCheckpoints[pos - 1].fromBlock == blockNumber) { quorumParamsCheckpoints[pos - 1].params = params; } else { quorumParamsCheckpoints.push(DynamicQuorumParamsCheckpoint({ fromBlock: blockNumber, params: params })); } } When calling the following _setMinQuorumVotesBPS, _setMaxQuorumVotesBPS, _setQuorumCoefficient, and _setDynamicQuorumParams functions, _writeQuorumParamsCheckpoint is called to record the updated dynamic quorum votes paramters at the relevant block. function _setMinQuorumVotesBPS(uint16 newMinQuorumVotesBPS) external { require(msg.sender == admin, 'NounsDAO::_setMinQuorumVotesBPS: admin only'); DynamicQuorumParams memory params = getDynamicQuorumParamsAt(block.number); require( newMinQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS_LOWER_BOUND && newMinQuorumVotesBPS <= MIN_QUORUM_VOTES_BPS_UPPER_BOUND, 'NounsDAO::_setMinQuorumVotesBPS: invalid min quorum votes bps' ); require( newMinQuorumVotesBPS <= params.maxQuorumVotesBPS, 'NounsDAO::_setMinQuorumVotesBPS: min quorum votes bps greater than max' ); uint16 oldMinQuorumVotesBPS = params.minQuorumVotesBPS; params.minQuorumVotesBPS = newMinQuorumVotesBPS; _writeQuorumParamsCheckpoint(params); emit MinQuorumVotesBPSSet(oldMinQuorumVotesBPS, newMinQuorumVotesBPS); } function _setMaxQuorumVotesBPS(uint16 newMaxQuorumVotesBPS) external { require(msg.sender == admin, 'NounsDAO::_setMaxQuorumVotesBPS: admin only'); DynamicQuorumParams memory params = getDynamicQuorumParamsAt(block.number); require( newMaxQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS_UPPER_BOUND, 'NounsDAO::_setMaxQuorumVotesBPS: invalid max quorum votes bps' ); require( params.minQuorumVotesBPS <= newMaxQuorumVotesBPS, 'NounsDAO::_setMaxQuorumVotesBPS: min quorum votes bps greater than max' ); uint16 oldMaxQuorumVotesBPS = params.maxQuorumVotesBPS; params.maxQuorumVotesBPS = newMaxQuorumVotesBPS; _writeQuorumParamsCheckpoint(params); emit MaxQuorumVotesBPSSet(oldMaxQuorumVotesBPS, newMaxQuorumVotesBPS); } function _setQuorumCoefficient(uint32 newQuorumCoefficient) external { require(msg.sender == admin, 'NounsDAO::_setQuorumCoefficient: admin only'); DynamicQuorumParams memory params = getDynamicQuorumParamsAt(block.number); uint32 oldQuorumCoefficient = params.quorumCoefficient; params.quorumCoefficient = newQuorumCoefficient; _writeQuorumParamsCheckpoint(params); emit QuorumCoefficientSet(oldQuorumCoefficient, newQuorumCoefficient); } function _setDynamicQuorumParams( uint16 newMinQuorumVotesBPS, uint16 newMaxQuorumVotesBPS, uint32 newQuorumCoefficient ) public { if (msg.sender != admin) { revert AdminOnly(); } if ( newMinQuorumVotesBPS < MIN_QUORUM_VOTES_BPS_LOWER_BOUND || newMinQuorumVotesBPS > MIN_QUORUM_VOTES_BPS_UPPER_BOUND ) { revert InvalidMinQuorumVotesBPS(); } if (newMaxQuorumVotesBPS > MAX_QUORUM_VOTES_BPS_UPPER_BOUND) { revert InvalidMaxQuorumVotesBPS(); } if (newMinQuorumVotesBPS > newMaxQuorumVotesBPS) { revert MinQuorumBPSGreaterThanMaxQuorumBPS(); } DynamicQuorumParams memory oldParams = getDynamicQuorumParamsAt(block.number); DynamicQuorumParams memory params = DynamicQuorumParams({ minQuorumVotesBPS: newMinQuorumVotesBPS, maxQuorumVotesBPS: newMaxQuorumVotesBPS, quorumCoefficient: newQuorumCoefficient }); _writeQuorumParamsCheckpoint(params); emit MinQuorumVotesBPSSet(oldParams.minQuorumVotesBPS, params.minQuorumVotesBPS); emit MaxQuorumVotesBPSSet(oldParams.maxQuorumVotesBPS, params.maxQuorumVotesBPS); emit QuorumCoefficientSet(oldParams.quorumCoefficient, params.quorumCoefficient); } By knowing the latest block that has the dynamic quorum votes paramters recorded for, a user can create a proposal with these parameters in mind. For example, if the latest block that has the dynamic quorum votes parameters recorded for is Block 500, the user can expect the proposal to be associated with these parameters when creating it in Block 505. However, if another proposal (Proposal B) that proposes to update the dynamic quorum votes parameters is executed after the proposal (Proposal A) that is just created in the same block, such as Block 505 in the example above, the dynamic quorum votes parameters associated with Proposal A are changed according to Proposal B. This is unexpected and unfair to the user who creates Proposal A in which Proposal A that could pass if using the original parameters can fail to pass when voting is done with the changed parameters. If the affected proposal is for an urgent action that transfers the ETH balance or fixing a critical bug by updating a contract for the protocol, the consequence can be huge. ## Proof of Concept First, please import minerStart, minerStop, and mineBlock in test\governance\NounsDAO\V2\end2end.test.ts as follows. import { deployNounsToken, deployWeth, address, encodeParameters, advanceBlocks, blockTimestamp, setNextBlockTimestamp, blockNumber, populateDescriptorV2, minerStart, // @audit minerStart is imported for testing purpose minerStop, // @audit minerStop is imported for testing purpose mineBlock // @audit mineBlock is imported for testing purpose } from '../../../utils'; Then, please append the following test in the then the logic upgrades to V2 and dynamic quorum is set describe block in test\governance\NounsDAO\V2\end2end.test.ts. This test should pass to demonstrate the described scenario. it("dynamic quorum votes parameters for a proposal (Proposal A) are changed according to another proposal (Proposal B) that proposes to update dynamic quorum votes parameters when Proposal B is executed after Proposal A is created in the same block", async () => { // for this test, proposalId4 is Proposal B and proposalId5 is Proposal A upgradeToV2ProposalId = await proposeUpgradeToV2AndSetDynamicQuorum(bidders[1]); await executeQueuedProposal(upgradeToV2ProposalId); await advanceBlocks(1); const govV2 = NounsDaoLogicV2Factory.connect(gov.address, deployer); // dynamic quorum votes parameters are set according to the proposeUpgradeToV2AndSetDynamicQuorum function that is called above let dynamicQuorumParams = await govV2.getDynamicQuorumParamsAt(await blockNumber()); expect(dynamicQuorumParams.minQuorumVotesBPS).to.equal(minQuorumVotesBPS); expect(dynamicQuorumParams.maxQuorumVotesBPS).to.equal(maxQuorumVotesBPS); expect(dynamicQuorumParams.quorumCoefficient).to.equal(quorumCoefficient); // bidders[4] proposes to call _setDynamicQuorumParams for updating dynamic quorum votes parameters let targets = [govV2.address]; let values = [0]; let signatures = ['_setDynamicQuorumParams(uint16,uint16,uint32)']; let callDatas = [ encodeParameters( ['uint16', 'uint16', 'uint32'], [minQuorumVotesBPS + 10, maxQuorumVotesBPS + 100, quorumCoefficient.add(1)], ) ]; await govV2.connect(bidders[4]).propose(targets, values, signatures, callDatas, '', {gasLimit: 1e6}); const proposalId4 = await govV2.latestProposalIds(bidders[4].address); await advanceBlocks(VOTING_DELAY + 1); // voting for proposalId4 await govV2.connect(bidders[0]).castVote(proposalId4, 0); await govV2.connect(bidders[1]).castVote(proposalId4, 0); await govV2.connect(bidders[2]).castVote(proposalId4, 1); await govV2.connect(bidders[3]).castVote(proposalId4, 1); await govV2.connect(bidders[4]).castVote(proposalId4, 1); await govV2.connect(bidders[5]).castVote(proposalId4, 1); await advanceBlocks(VOTING_PERIOD); // proposalId4 succeeds expect(await govV2.state(proposalId4)).to.equal(4); // proposalId4 is queued await govV2.connect(bidders[4]).queue(proposalId4); expect(await govV2.state(proposalId4)).to.equal(5); const proposal4 = await govV2.proposals(proposalId4); await setNextBlockTimestamp(proposal4.eta.toNumber(), false); // bidders[3] creates a proposal targets = [address(3)]; values = [0]; signatures = ['']; callDatas = ['0x']; await govV2.connect(bidders[3]).propose(targets, values, signatures, callDatas, 'proposal', {gasLimit: 1e6}); const proposalId3 = await govV2.latestProposalIds(bidders[3].address); const proposal3 = await govV2.proposals(proposalId3); // proposalId3's creationBlock is current block number before checking dynamic quorum votes parameters at the current block expect(proposal3.creationBlock).to.be.eq(await blockNumber()); // dynamic quorum votes parameters are same as before dynamicQuorumParams = await govV2.getDynamicQuorumParamsAt(await blockNumber()); expect(dynamicQuorumParams.minQuorumVotesBPS).to.equal(minQuorumVotesBPS); expect(dynamicQuorumParams.maxQuorumVotesBPS).to.equal(maxQuorumVotesBPS); expect(dynamicQuorumParams.quorumCoefficient).to.equal(quorumCoefficient); // proposalId3's creationBlock is current block number after checking dynamic quorum votes parameters at the current block expect(proposal3.creationBlock).to.be.eq(await blockNumber()); // begin to run transactions in same block await minerStop(); // bidders[5] creates an proposal before proposalId4's execution targets = [address(5)]; values = [0]; signatures = ['']; callDatas = ['0x']; await govV2.connect(bidders[5]).propose(targets, values, signatures, callDatas, 'proposal', {gasLimit: 1e6}); // in the same block, proposalId4 is executed after bidders[5] creates the proposal await govV2.execute(proposalId4, {gasLimit: 1e6}); await mineBlock(); // proposalId5's creationBlock is current block number before checking dynamic quorum votes parameters at the current block const proposalId5 = await govV2.latestProposalIds(bidders[5].address); const proposal5 = await govV2.proposals(proposalId5); expect(proposal5.creationBlock).to.be.eq(await blockNumber()); // dynamic quorum votes parameters are updated according to proposalId4, // which indicates that the dynamic quorum votes parameters for proposalId5 are adjusted after proposalId4's execution that happens after proposalId5's creation in the same block dynamicQuorumParams = await govV2.getDynamicQuorumParamsAt(await blockNumber()); expect(dynamicQuorumParams.minQuorumVotesBPS).to.equal(minQuorumVotesBPS + 10); expect(dynamicQuorumParams.maxQuorumVotesBPS).to.equal(maxQuorumVotesBPS + 100); expect(dynamicQuorumParams.quorumCoefficient).to.equal(quorumCoefficient.add(1)); // proposalId5's creationBlock is current block number after checking dynamic quorum votes parameters at the current block expect(proposal5.creationBlock).to.be.eq(await blockNumber()); await minerStart(); }); ## Tools Used VSCode ## Recommended Mitigation Steps When creating a proposal, the latest dynamic quorum votes parameters that are currently in effect should be recorded in the Proposal struct for this proposal. Later, when calculating the dynamic quorum votes for this proposal, these recorded parameters should be used. --- The text was updated successfully, but these errors were encountered: All reactions