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

10 stars 6 forks source link

Upgraded Q -> M from 399 [1664289734798] #731

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

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

GalloDaSballo commented 1 year ago

possible dos with uints and uint32 and block.timestamp since snapshot is uint256 and is unchecked the value where block.timestamp is large and voting delay is large and so the uint32 is overflowed but since its uint256 its ok but then when its casting it overflows which makes it zero which can cause loss of funds because the VoteStart and voteEnd would be zero and which which shouldnt happen uint256 snapshot; uint256 deadline;

    // Cannot realistically overflow
    unchecked {
        // Compute the snapshot and deadline
        snapshot = block.timestamp + settings.votingDelay;
        deadline = snapshot + settings.votingPeriod;
    }
    // Store the proposal data
    proposal.voteStart = uint32(snapshot);
    proposal.voteEnd = uint32(deadline);
    proposal.proposalThreshold = uint32(currentProposalThreshold);
    proposal.quorumVotes = uint32(quorum());
    proposal.proposer = msg.sender;
    proposal.timeCreated = uint32(block.timestamp);

https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L149-L166

reccomend to make sure that deadline and snapshot < uint32(max) or make it bigger types like uint96 to make it more future proof.

Dup of #482