code423n4 commented 1 year ago

Lines of code

https://github.com/arbitrumfoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/governors/modules/SecurityCouncilMemberElectionGovernorCountingUpgradeable.sol#L115-L118 https://github.com/arbitrumfoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/governors/modules/SecurityCouncilMemberElectionGovernorCountingUpgradeable.sol#L231-L234

Vulnerability details

Impact

In SecurityCouncilMemberElectionGovernorCountingUpgradeable.sol we have _countVote() function. This function registers a vote by some account for a nominee. It is more important to see the following code from this function:

uint240 weight = votesToWeight(proposalId, block.number, votes);
        if (weight == 0) {
            revert ZeroWeightVote(block.number, votes);
        }

We call the function votesToWeight() and if weight is zero the function revert.

votesToWeight returns the weight of a vote for a given proposal. Check the following code block:

uint256 startBlock = proposalSnapshot(proposalId);
        if (blockNumber <= startBlock) {   
            return 0;
        }

If blockNumber is equal to startBlock, the function will return 0, which implies that the votes have zero weight at the exact moment when the voting starts. But this is also the block in which voting begins. Because of this, it is impossible to vote right after voting starts.

Proof of Concept

Check this POC which proves that if someone votes right after voting starts will return zero because the function will think voting has not started.

function testVotesToWeight() public {
        uint256 proposalId = _propose(0);

        uint256 startBlock = governor.proposalSnapshot(proposalId);

        assertEq(
            governor.votesToWeight(proposalId, startBlock, 100), 0, "return zero"
        );
   }

Tools Used

Visual Studio Code

Recommended Mitigation Steps

Change:

uint256 startBlock = proposalSnapshot(proposalId);
        if (blockNumber <= startBlock) {   
            return 0;
        }

To:

uint256 startBlock = proposalSnapshot(proposalId);
        if (blockNumber < startBlock) {   
            return 0;
        }

Assessed type

Timing

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #188

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)