Open Dollar - Kaysoft's results

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/gov/ODGovernor.sol#L41 https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/gov/ODGovernor.sol#L31-L41

Vulnerability details

Impact

It is practically impossible for enough users to vote within a voting period of 4 seconds from the voting start time.

Proof of Concept

The problem lies in the votingDelay and votingPeriod that are set in the ODGovernor.sol this way.

contract ODGovernor is
  Governor,
  GovernorSettings,
  GovernorCompatibilityBravo,
  GovernorVotes,
  GovernorVotesQuorumFraction,
  GovernorTimelockControl
{
...
constructor(
    address _token,
    TimelockController _timelock
  )
    Governor('ODGovernor')//@audit larger voting delay gives users the time to unstake tokens if necessary.
    GovernorSettings(1, 15, 0)//@audit voting period 15blocks too small and no function to update. quorum will never be reached.
    GovernorVotes(IVotes(_token))
    GovernorVotesQuorumFraction(3)
    GovernorTimelockControl(_timelock)
  {}//@audit votingPeriod: how long does a proposal remain open to votes.
...
}

In the constructor above the GovernorSettings constructor is used to set the votingDelay and votingPeriod.

GovernorSettings(initialVotingDelay, initialVotingPeriod, initialProposalThreshold) measured in blocks
GovernorSettings(1, 15, 0)

The voting delay and voting period are measured in blocks. Since this project is built `specifically for Arbitrum, we will consider Arbitrum particularly to set this values.

• The average blocktime on arbitrum is 0.26 seconds.
• so setting 1 voting delay in the code above implies that there is just 0.26 seconds delay from proposal to voting start time.
• And setting 15 voting period above implies that users have just 15 * 0.26 = 3.9 seconds to cast their vote.

It will be more practical to set atleast 1 day voting delay and atleast 1 week voting period as set in OZ governance examples. To convert these times to blocks on Arbitrum based on 0.26 secs avg blocktime: 1 day = 86400 / 0.26 = ~332, 308 blocks per day so 1 week will be 332, 308 * 7 = 2,326,156 blocks per week

so it will be more practical for GovernorSettings parameters to be set this way:

//votingDelay = 1day and votingPeriod = 1week.
GovernorSettings(332308, 2326156, 0); //particulary for Arbitrum based on block numbers.

Calculation reference from OZ docs: https://docs.openzeppelin.com/contracts/4.x/governance#governor

In the inherited OZ's Governor.sol, the voting delay and voting period are used to set each proposal's voting start time and deadline in the propose function this way.

function propose(
        address[] memory targets,
        uint256[] memory values,
        bytes[] memory calldatas,
        string memory description
    ) public virtual override returns (uint256) {
...
 ProposalCore storage proposal = _proposals[proposalId];
 require(proposal.voteStart.isUnset(), "Governor: proposal already exists");
uint64 snapshot = block.number.toUint64() + votingDelay().toUint64();//@audit voting delay.
        uint64 deadline = snapshot + votingPeriod().toUint64();

        proposal.voteStart.setDeadline(snapshot);
        proposal.voteEnd.setDeadline(deadline);
...
}

Tools Used

Openzeppelin Governance docs, OZ's smart contract wizard. https://docs.openzeppelin.com/contracts/4.x/governance#governor

Recommended Mitigation Steps

• Short term: Use Oz's examples but do your calculations based on Arbitrum block time:

GovernorSettings(332308, 2326156, 0); //particulary for Arbitrum based on block numbers.

OZ's wizard example: https://wizard.openzeppelin.com/#governor

• Long term: Use latest Oz's from 4.9 and above that have time based voting instead of block number. read here: https://docs.openzeppelin.com/contracts/4.x/governance#disclaimer

Assessed type

Governance