Nouns Builder contest - joestakey's results

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L473-L477

Vulnerability details

quorum() currently rounds down, as per Solidity truncation happening in division. In the case of Nouns, where a DAO can have a Token with a low supply and decimals = 0, this means the number of votes required for a proposal can be 1 vote lower than expected.

Impact

Medium

Proof Of Concept

quorum is calculated as follow:

472: /// @notice The current number of votes required to be in favor of a proposal in order to reach quorum
473:     function quorum() public view returns (uint256) {
474:         unchecked {
475:             return (settings.token.totalSupply() * settings.quorumThresholdBps) / 10_000;
476:         }
477:     }

Assume that settings.quorumThresholdBps == 1000 (10%) and settings.token.totalSupply() == 9

quorum = 9 * 1000 / 10_000 = 9_000 / 10_000 = 0.

This means no minimum number of votes in support of a proposal for it to succeed.

When calling propose, proposal.quorumVotes will hence be zero.

This means one vote is enough for a proposal to be successful

Tools Used

Manual Analysis

Mitigation

You can use the modulo operator to ensure that:

• the quorum will always be greater than 1
• quorum will always be rounded up instead of down.

-             return (settings.token.totalSupply() * settings.quorumThresholdBps) / 10_000;
+             uint256 supplyThreshold = settings.token.totalSupply() * settings.quorumThresholdBps;
+             return supplyThreshold % 10_000 == 0 ? supplyThreshold / 10_000 : supplyThreshold / 10_000 + 1;