Divide before multiply may lead to loss of precision

[code423n4]

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/party/PartyGovernance.sol#L1062 https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/party/PartyGovernance.sol#L1069 https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/distribution/TokenDistributor.sol#L263 https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/party/PartyGovernance.sol#L1062

Vulnerability details

Impact

Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision.

Details

This would affect the dynamicQuorumVotes vote logic correct functioning

Proof of Concept contracts/distribution/TokenDistributor.sol:263 contracts/distribution/TokenDistributor.sol:352

258:                (
259:                 uint256(party.getDistributionShareOf(partyTokenId))
260:                 * memberSupply
261:                + (1e18 - 1)
262:                 )
263:                / 1e18

  352:         uint128 fee = supply * args.feeBps / 1e4;

contracts/party/PartyGovernance.sol#L1062 contracts/party/PartyGovernance.sol#L1089

  1062:         uint256 acceptanceRatio = (totalVotes * 1e4) / totalVotingPower;

  1078            return uint256(voteCount) * 1e4
  1079:             / uint256(totalVotingPower) >= uint256(passThresholdBps);

contracts/party/PartyGovernanceNFT.sol

  112:         return votingPowerByTokenId[tokenId] * 1e18 / _getTotalVotingPower();

Recommended Mitigation Steps

Consider adding bracket in multiplication numbers. Example:

Now:
uint128 fee = supply * args.feeBps / 1e4;

Fixed:
uint128 fee = (supply * args.feeBps) / 1e4;

[merklejerk]

Duplicate of #303

[merklejerk]

Some of these examples point to areas where grouping syntax is already used.

[HardlyDifficult]

& they all appear to be dividing after multiply.. closing as invalid.