Division before Multiplication could incur unnecessary precision loss

[code423n4]

Lines of code

https://github.com/nounsDAO/nouns-monorepo/blob/718211e063d511eeda1084710f6a682955e80dcb/packages/nouns-contracts/contracts/governance/NounsDAOV3DynamicQuorum.sol#L64

Vulnerability details

Impact

Division before multiplication could incur unnecessary precision loss causing loss of funds.

Proof of Concept

In the dynamicQuorumVotes() function of NounsDAOV3DynamicQuorum.sol contract the value of againstVotesBPS is first calculated by dividing a value by totalSupply before doing another multiplication with againstVotesBPS in the next line.

function dynamicQuorumVotes(
        uint256 againstVotes,
        uint256 totalSupply,
        NounsDAOStorageV3.DynamicQuorumParams memory params
    ) public pure returns (uint256) {
@>        uint256 againstVotesBPS = (10000 * againstVotes) / totalSupply; //@audit div first
@>     uint256 quorumAdjustmentBPS = (params.quorumCoefficient * againstVotesBPS) / 1e6; //@audit then mul 
        uint256 adjustedQuorumBPS = params.minQuorumVotesBPS + quorumAdjustmentBPS;
        uint256 quorumBPS = min(params.maxQuorumVotesBPS, adjustedQuorumBPS);
        return bps2Uint(quorumBPS, totalSupply);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

When implementing mathematical operations involving multiplication and division, always ensure all division come last after all multiplications have been implemented to avoid incurring precision loss and subsequent loss of value.

Assessed type

Math

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #194

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid