sherlock-audit / 2023-10-mzero-judging

3 stars 2 forks source link

0xhashiman - Quorum Value will always be 1 #70

Closed sherlock-admin4 closed 5 months ago

sherlock-admin4 commented 5 months ago

0xhashiman

high

Quorum Value will always be 1

Summary

The StandardGovernor is a governance contract, users are able to propose,vote and execute tranasactions when they meet all requirements. However an issue exists with the current implementation since the project hardcoded the value of quorom to be 1, in StandardGovernor contract we can also check BatchGovernor to verify that there is no function to override this value (since StandardGovernor inherits BatchGovernor). The function quorum() which suppose to returns the minimum number of eligible (COUNTING_MODE) votes for a proposal to succeed at timepoint. gives 1 since its hardcoded as you see in the code snippet.

Vulnerability Detail

The vulnerability exists since their is no other functions to override this value and the project is not taking into consideration if votes counting reached quorum or no which is one of the most important things in governance contract. To verify this here is the code of the state() function that gives whether a proposal is Succeeded or Defeated.

    function state(uint256 proposalId_) public view override(BatchGovernor, IGovernor) returns (ProposalState) {
        ...
        ..
        if (currentEpoch_ == voteEnd_) return ProposalState.Active;

        if (proposal_.yesWeight <= proposal_.noWeight) return ProposalState.Defeated; //@audit we dont check if the propoposal weight reached the quorum or no 
        unchecked {
            return (currentEpoch_ <= voteEnd_ + 1) ? ProposalState.Succeeded : ProposalState.Expired;
        }
    }

When we add both of the issues 1) hardcoding the value of quorum and 2) not taking into consideration if the weight of the votes reached quorum this can lead to users proposing and voting any arbitrary proposal and it will be valid.

Impact

In this case when someone propose if his proposal got 3 votes in yesWeight and 2 votes in noWeight, this will be a valid proposal because the contract is not implementing the quorum check. But normally in this case the contract should revert saying the voting count didnt reach the quorum and mark the proposal as defeated

Code Snippet

StandardGovernor quorum() https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/StandardGovernor.sol#L270-L272 StandardGovernor state() https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/StandardGovernor.sol#L280-L301

Tool used

Manual Review

Recommendation

Instead of hardcoding the value of quorum make the deployer set it up in the constructor and in the state function make sure to verify if the weight of the vote is greater tha quorum, if not we can mark the proposal as defeated.

sherlock-admin3 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

but that is just returning the proposal state which in this case quorum is not required.

nevillehuang commented 5 months ago

Invalid, known issue see Three Sigma Report L-09