code-423n4 / 2022-08-nounsdao-findings

2 stars 0 forks source link

`dynamicQuorumVotes()` can be calcualted incorrectly #291

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L909-L912

Vulnerability details

[M-00] dynamicQuorumVotes() can be calcualted incorrectly

Problem

Incorrect values of params.quorumCoefficient impact dynamic quorum votes calculation. When quorumCoefficient is set there is no sanity checking if this value is correct, which is >= 1e6 and < 1e7

contracts/governance/NounsDAOLogicV2.sol
726:     function _setQuorumCoefficient(uint32 newQuorumCoefficient) external {
727:         require(msg.sender == admin, 'NounsDAO::_setQuorumCoefficient: admin only');
728:         DynamicQuorumParams memory params = getDynamicQuorumParamsAt(block.number);
729: 
730:         uint32 oldQuorumCoefficient = params.quorumCoefficient;
731:         params.quorumCoefficient = newQuorumCoefficient;

Proof of Concept

contracts/governance/NounsDAOLogicV2.sol
909:         uint256 quorumAdjustmentBPS = (params.quorumCoefficient * againstVotesBPS) / 1e6;
910:         uint256 adjustedQuorumBPS = params.minQuorumVotesBPS + quorumAdjustmentBPS;

if quorumCoefficient is low enough it can result in quorumAdjustmentBPS to become zero if quorumCoefficient is high enough, the quorumBPS will always be equal params.maxQuorumVotesBPS and always ignore adjustedQuorumBPS

Mitigation

Add sanity check in _setQuorumCoefficient(uint32 newQuorumCoefficient): if (newQuorumCoefficient >= 1e7 || newQuorumCoefficient < 1e6) revert IncorrectQuorumCoefficient()

davidbrai commented 2 years ago

Duplicate of #397