code-423n4 / 2022-09-party-findings

2 stars 0 forks source link

Invalid Use of BODMAS in TokenDistributor.sol and PartyGovernanceNFT.sol #303

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/distribution/TokenDistributor.sol#L352 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/party/PartyGovernanceNFT.sol#L112

Vulnerability details

Invalid Use of BODMAS in TokenDistributor.sol and PartyGovernanceNFT.sol

In contract TokenDistributor.sol we initializing the value of uint128 fee using a mathematical way but in a wrong manner their brackets are missing which will clear out the instance of using it since it seems like we multiplying before division which is not possible but their is possibility that first division will occur and afterwards multiplication happens but it is not clearly mention out from here which can create an isssue and their is somewhat the same thing happening in PartyGovernanceNFT.sol where we are returning some value .

Instances

TokenDistributor.sol

contracts/distribution/TokenDistributor.sol:352:        uint128 fee = supply * args.feeBps / 1e4;
            _storedBalances[balanceId] = args.currentTokenBalance;
        }

        // Create a distribution.
        uint128 fee = supply * args.feeBps / 1e4;
        uint128 memberSupply = supply - fee;

        info = DistributionInfo({
            tokenType: args.tokenType,
            distributionId: ++lastDistributionIdPerParty[args.party],
            token: args.token,

PartyGovernanceNFT.sol

contracts/party/PartyGovernanceNFT.sol:112:        return votingPowerByTokenId[tokenId] * 1e18 / _getTotalVotingPower();
   }

    /// @inheritdoc ITokenDistributorParty
    function getDistributionShareOf(uint256 tokenId) external view returns (uint256) {
        return votingPowerByTokenId[tokenId] * 1e18 / _getTotalVotingPower();
    }

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add brackets at the correct place to have value added calculation

merklejerk commented 1 year ago

Solidity is already evaluating the math operations in order, so the multiplications always happen before the division. There is no need for grouping syntax.