code-423n4 / 2022-09-nouns-builder-findings

10 stars 6 forks source link

Upgraded Q -> M from 409 [1664289656223] #727

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Judge has assessed an item in Issue #409 as Medium risk. The relevant finding follows:

GalloDaSballo commented 1 year ago

Token.sol _addFounders() Need to check ownershipPct <100,don't check uint8(founderPct) after conversion to compare

    function _addFounders(IManager.FounderParams[] calldata _founders) internal {
        // Cache the number of founders
        uint256 numFounders = _founders.length;

        // Used to store the total percent ownership among the founders
        uint256 totalOwnership;

        unchecked {
            // For each founder:
            for (uint256 i; i < numFounders; ++i) {
                // Cache the percent ownership
                uint256 founderPct = _founders[i].ownershipPct;

                // Continue if no ownership is specified
                if (founderPct == 0) continue;

                // Update the total ownership and ensure it's valid
---             if ((totalOwnership += uint8(founderPct)) > 100) revert INVALID_FOUNDER_OWNERSHIP();
+++             if ((totalOwnership += founderPct) > 100) revert INVALID_FOUNDER_OWNERSHIP();
GalloDaSballo commented 1 year ago

Dup of #303