Summary

The GovernorCountingFractional.sol's quorum logic is broken cause it attempts to only count for votes for quorum but wrongly specifies it in the COUNTING MODE

Vulnerability Details

First note that the L2 contracts attempt to extend the Governor.sol in order to now support 3 options faction for vote counting.

This approach is also heavily inspired from Scopelift's scope, see

https://github.com/Cyfrin/2024-10-zksync/blob/cfc1251de29379a9548eeff1eea3c78267288356/zk-governance/l2-contracts/src/lib/GovernorCountingFractional.sol#L21-L25

Now going to the implementation of GovernorCountingFractional we can see that whereas Zksync claim this code has been copied there are some differences which we can see, for e.g in the implemented approach, only for votes are counted to quorum, i.e:

https://github.com/Cyfrin/2024-10-zksync/blob/cfc1251de29379a9548eeff1eea3c78267288356/zk-governance/l2-contracts/src/lib/GovernorCountingFractional.sol#L89-L98

This is unlike Scopelift's scope where both for and abstain votes are counted towards quorum:

https://github.com/ScopeLift/flexible-voting/blob/d71c4e02256d4ef6c9067b947d398753402afdd2/src/GovernorCountingFractional.sol#L90C1-L98C1

The above reason is why in scope lift's case the COUNTING_MODE was specified as such:

https://github.com/ScopeLift/flexible-voting/blob/d71c4e02256d4ef6c9067b947d398753402afdd2/src/GovernorCountingFractional.sol#L49C1-L55C6

Notice the "support=bravo&quorum=for,abstain&params=fractional"

i.e:

• support=bravo

• quorum=for,abstain, and

• params=fractional

This is correctly implemented, cause when we go to the official Openzeppelin documentation where this all has been inherited from we see the documentation below:

https://docs.openzeppelin.com/contracts/4.x/api/governance#IGovernor-COUNTING_MODE--

COUNTING_MODE() → string - public

A description of the possible support values for castVote and the way these votes are counted, meant to be consumed by UIs to show correct vote options and interpret the results. The string is a URL-encoded sequence of key-value pairs that each describe one aspect, for example support=bravo&quorum=for,abstain.

There are 2 standard keys: support and quorum.

support=bravo refers to the vote options 0 = Against, 1 = For, 2 = Abstain, as in GovernorBravo.

quorum=bravo means that only For votes are counted towards quorum.

quorum=for,abstain means that both For and Abstain votes are counted towards quorum.

If a counting module makes use of encoded params, it should include this under a params key with a unique name that describes the behavior. For example:

params=fractional might refer to a scheme where votes are divided fractionally between for/against/abstain.

params=erc721 might refer to a scheme where specific NFTs are delegated to vote.

Evidently, we can see that whereas Scopelift correctly implements the options, i.e support=bravo cause they want the three options (for, against and abstain) and also quorum=for,abstain cause they count both for and abstain votes to quorum, see previously tagged Scopelift's snippet.

However in the case for Zksync we should have the quorum option to be bravo cause only the for votes are counted against quorum but this si wrongly implemented cause we have quorum=for instead breaking the logic

See it in code snippets:

https://github.com/Cyfrin/2024-10-zksync/blob/cfc1251de29379a9548eeff1eea3c78267288356/zk-governance/l2-contracts/src/lib/GovernorCountingFractional.sol#L56-L59

Impact

The counting mode against the quorum is wrong leading to wrong accounting of votes as the mode does not align with what's been used tor each quorum and breaks all external integration since the correct vote options can't be shown neither would the results be correctly shown.

Tools Used

Manual review

Recommendations

Apply these changes: