Nouns Builder contest - GimelSec's results

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/governance/governor/Governor.sol#L362-L364

Vulnerability details

Impact

Anyone can create a proposal if his/her voting weight is greater than or equal to the proposal threshold. But if the proposer's voting weight is equal to the threshold, anyone can cancel this proposal.

Proof of Concept

In propose function, it ensures voting weight is greater than or equal to the proposal threshold:

            // Ensure the caller's voting weight is greater than or equal to the threshold
            if (getVotes(msg.sender, block.timestamp - 1) < proposalThreshold()) revert BELOW_PROPOSAL_THRESHOLD();

But if the proposer's voting weight is equal to the threshold, anyone can call cancel due to this condition:

            // Ensure the caller is the proposer or the proposer's voting weight has dropped below the proposal threshold
            if (msg.sender != proposal.proposer && getVotes(proposal.proposer, block.timestamp - 1) > proposal.proposalThreshold)
                revert INVALID_CANCEL();

Because the proposer’s voting weight is equal to the proposal.proposalThreshold, it will not be reverted and call cancel successfully.

Tools Used

None

Recommended Mitigation Steps

L363 should use >= rather than >:

            if (msg.sender != proposal.proposer && getVotes(proposal.proposer, block.timestamp - 1) >= proposal.proposalThreshold)
                revert INVALID_CANCEL();

Summary

We list 1 low-critical finding:

• (Low) It’s better to define uint8 founderPct

(Low) It’s better to define uint8 founderPct

Impact

founderPct is defined as uint256, but it’s used for both uint8 and uint256.

Proof of Concept

founderPct is uint256 but is used by uin8: https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/token/Token.sol#L82 https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/token/Token.sol#L88

                uint256 founderPct = _founders[i].ownershipPct;
                if ((totalOwnership += uint8(founderPct)) > 100) revert INVALID_FOUNDER_OWNERSHIP();

But L102 uint256 again: https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/token/Token.sol#L102

                uint256 schedule = 100 / founderPct;

Recommended Mitigation Steps

Define uint8 rather than uint256 in L82.