114.md

Odd Plum Crab

Medium

Proposals can not be executed even though they have received 2/3 of the voting power

Summary

For a proposal to be executable is required that is accumulates a 2/3 of the voting power in favor of the proposal, otherwise the proposal is not executable and may expire.

There is a problem when validating if the 2/3 of voting power have been reached on proposals. The usage of the incorrect operator causes that the voting power required to execute a proposal to be > 2/3 of the total voting power.

function _checkPassed(uint256 proposalID) internal view returns (bool) {
    // checking invalidate votes
    address[] memory latestSequencerSet = ISequencer(SEQUENCER_CONTRACT).getSequencerSet2();
    uint256 validVotes = 0;
    for (uint256 i = 0; i < latestSequencerSet.length; i++) {
        if (votes[proposalID].contains(latestSequencerSet[i])) {
            validVotes = validVotes + 1;
        }
    }
    //@audit-issue => Using `>` forces the voting power to be greather than 2/3. For a set of 3 sequencers, it would require a 100% of the voting power.
    return validVotes > (latestSequencerSet.length * 2) / 3;
}

• For example, if the sequencerSet is composed of 3 sequencers, 2/3 of the voting power would be 2 votes in favor. But because the incorrect operator, the voting power required to pass a proposal would actually be a 100%

Root Cause

Incorrect operator to validate if proposals have reached 2/3 of the voting power.

Internal pre-conditions

Proposal reach 2/3 of the voting power but they can't be executed anyways.

External pre-conditions

none

Attack Path

1. Proposal is created.
2. 2/3 of the voters vote in favor of the proposal
3. Attempt to execute the proposal, the execution reverts because the validation to check if 2/3 of the voting power has been reached is incorrect.

Impact

Proposals require more voting power than intended to be executed.

• Depending on the size of the sequencerSet, the voting power may even be a 100% (i.e. a sequencerSet of 3 sequencers).

PoC

No need of a coded PoC, please reffer to the example provided on the Summary section.

Mitigation

Update the operator used to determine if the 2/3 of the voting power has been reached.

• Instead of using >, use >=.

function _checkPassed(uint256 proposalID) internal view returns (bool) {
    // checking invalidate votes
    address[] memory latestSequencerSet = ISequencer(SEQUENCER_CONTRACT).getSequencerSet2();
    uint256 validVotes = 0;
    for (uint256 i = 0; i < latestSequencerSet.length; i++) {
        if (votes[proposalID].contains(latestSequencerSet[i])) {
            validVotes = validVotes + 1;
        }
    }
+   return validVotes >= (latestSequencerSet.length * 2) / 3;
-   return validVotes > (latestSequencerSet.length * 2) / 3;
}