search

code-423n4 / 2021-10-union-findings

0 stars 0 forks source link

Governor contract is not matching Contract source #2

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

csanuragjain

Vulnerability details

Impact

Governor documentation requires :

  1. more than 10% of the current supply of Union token to propose changes
  2. Voting period of 3 days

Both of this condition are not fulfil in the contract

Proof of Concept

  1. Lets navigate to contract at https://github.com/code-423n4/2021-10-union/blob/main/contracts/governance/UnionGovernor.sol

  2. Check the function votingPeriod

function votingPeriod() public pure virtual override returns (uint256) {
        return 46027; // 1 week
    }

  1. As we can see contract is setting votingPeriod of 1 week even though documentation mention this to be 3 days which is wrong

  2. Now lets check the propose function

   function propose(
        address[] memory targets,
        uint256[] memory values,
        bytes[] memory calldatas,
        string memory description
    ) public override(IGovernor, Governor, GovernorCompatibilityBravo) returns (uint256) {
        _checkUserLatestProposal();
        proposalCount++;
        uint256 proposalId = hashProposal(targets, values, calldatas, keccak256(bytes(description)));
        latestProposalIds[_msgSender()] = proposalId;
        return super.propose(targets, values, calldatas, description);
    }
  1. As we can there is no check to see if user has addresses with more than 10% of the current supply of Union token, which means any user can vote irrespective of the supply (function is public)

Recommended Mitigation Steps

Add the necessary steps to enforce the voting restriction and change the voting period to 3 days

kingjacob commented 3 years ago

Super.Propose is calling the inherited function which does have the proposal threshold check. So that's not an issue.

The voting period is low risk.

GalloDaSballo commented 3 years ago

Agree that the documentation mismatching the code is a valid finding, agree with severity of Low.

Let's verify the sponsor statement: Super.Propose is calling the inherited function which does have the proposal threshold check. So that's not an issue.

By going through the chain of inheritance we can verify that the voting threshold is indeed enforced: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/e63b09c9ad3a45484b6dc304e0e99640a9dc3036/contracts/governance/Governor.sol#L186