The ODGovernor is using the Governor with block.numbers which doesnt properly work on Arbitrum

[c4-submissions]

Lines of code

https://github.com/open-dollar/od-contracts/blob/f4f0246bb26277249c1d5afe6201d4d9096e52e6/src/contracts/gov/ODGovernor.sol#L10

Vulnerability details

The ODGovernor is using a OZ version of Governor where block.numbers is used but the problem is that block.number doesnt properly work on Arbitrum. As the docs mention, block.number returns the most recently synced L1 block number. Once per minute, the block number in the Sequencer is synced to the actual L1 block number. Using block.number in the Governor can lead to inaccurate timing.

Impact

Block.number is used in many places like when checking the deadline or snapshot however this will lead to inaccurate timing and different time periods than its supposed to be.

Proof of Concept

The ODGovernor is using Governor v4.8.0 and as you can see block.number is used which will make the deadline and snapshot innacurate.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/49c0e4370d0cc50ea6090709e3835a3091e33ee2/contracts/governance/Governor.sol#L265-L266

265:  uint64 snapshot = block.number.toUint64() + votingDelay().toUint64();
266:  uint64 deadline = snapshot + votingPeriod().toUint64();

Tools Used

Manual Review

Recommended Mitigation Steps

https://docs.openzeppelin.com/contracts/4.x/governance#timestamp_based_governance

Consider switching to the newer version where block.timestamp can be used by overriding the clock() and CLOCK_MODE() functions. Please note that the votingDelay() and votingPeriod() has to be set accordingly

Assessed type

Timing

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #17

[c4-judge]

MiloTruck marked the issue as unsatisfactory: Out of scope

[c4-judge]

MiloTruck marked the issue as not a duplicate

[c4-judge]

MiloTruck marked the issue as unsatisfactory: Invalid

[MiloTruck]

OZ's governance-related libraries using block.number is NOT a vulnerability, they work completely fine.