0xhashiman - Wrong value of _votingPeriod

[sherlock-admin2]

0xhashiman

high

Wrong value of _votingPeriod

Summary

The _votingPeriod() is a function that Returns the number of clock values between the vote start and vote end. However in both StandardGovernor and ThresholdGovernor this function will always return 0 in StandardGovernor and 1 in ThresholdGovernor since it is hardcoded, the problem of this is that this is an important value to determine the VoteEnd and with the current implementations the voting and the governance system will not work as it should.

Vulnerability Detail

Here is the _votingPeriod() in StandardGovernor:

  function _votingPeriod() internal pure override returns (uint16) {
        return 0; //@audit-issue hardcoding the voting period value 
    }

For StandardGovernor contract this function is used in BatchGovernor to determine the timepoint at which voting would end given a timepoint at which voting would start.

    function _getVoteEnd(uint16 voteStart_) internal view returns (uint16) {
        unchecked {
            return voteStart_ + _votingPeriod();
        }
    }

This means that the voteEnd will be the same as voteStart_ because the _votingPeriod() will always return 0. The value of voteEnd should not be equal to voteStart because we check if a proposal is valid by verifying if its between voteStart and voteEnd. There is also no function to override this value after deployment.

Impact

In both contract StandardGovernor and ThresholdGovernor the voting and all governance mechanisms wont work properly because the contract wont be able to determine the voting end since the votingPeriod is always 0. The ThresholdGovernor is heavily used in ZeroGovernor contract and there is also no way to modify this value.

Code Snippet

_votingPeriod() in StandardGovernor https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/StandardGovernor.sol#L468-L470

_getVoteEnd() that uses _votingPeriod() in BatchGovernor https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/abstract/BatchGovernor.sol#L633-L637

_votingPeriod() in ThresholdGovernor https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/abstract/ThresholdGovernor.sol#L179-L181

Tool used

Manual Review

Recommendation

Allow the deployer to setup the votingPeriod in constructor you can also add a function to override and change this value if needed.

[sherlock-admin4]

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

takarez commented:

this is just saying that voting can immediately start after creating the proposal, nothing more.

[nevillehuang]

Invalid, this is intended, voting period is custom to the protocol and designed to be compatible with epochs as seen here

[deluca-mike]

Invalid because by standards, votingPeriod() is the difference between the start epoch and the end epoch for voting. If they are the same epoch (i.e. can only vote for 1 epoch), that difference is 0.