Closed c4-bot-2 closed 5 months ago
0xSorryNotSorry marked the issue as sufficient quality report
0xSorryNotSorry marked the issue as duplicate of #816
Trumpero marked the issue as not a duplicate
Trumpero changed the severity to QA (Quality Assurance)
Trumpero marked the issue as grade-b
This previously downgraded issue has been upgraded by Trumpero
Trumpero marked the issue as duplicate of #816
Trumpero changed the severity to QA (Quality Assurance)
HI @Trumpero, this seems to have been wrongly duplicated to 816 and graded as QA, this issue states a different thing and is not about hardcoded block minting time, however stands on it's own and is an issue about the nuances of block.number
on arbitrum like L2s.
As hinted in report, this can be seen from the version 4.9 of Openzeppelin, where they modified the governor code to now use clocks in order to curb the inconsistencies that would otherwise be present.
Agree with @Bauchibred
Hardcoded block minting time and block.number
in arbitrum aren't same issue
we have tried to explain in #826
The decision to use block number instead of timestamp is intended to standardize the votes by blocks, as vote tokens have checkpoints of balances per block number, not timestamp. The number of blocks for a duration is estimated, and only the voting duration uses block numbers, which is long enough to be safe (7 days). Therefore, the only valuable information in this report is the different block times of different chains, which suggests that the protocol shouldn't hardcode the number of blocks for voting duration. Thus, this should be considered a duplicate of #816 and should be considered as low severity at most.
Lines of code
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/README.md?plain=1#L123
Vulnerability details
Impact
Now On Optimism, block.number is the L2 block number, but on Arbitrum, it's the L1 block number, and
ArbSys(address(100)).arbBlockNumber()
must be used. Furthermore, L2 block numbers often occur much more frequently than L1 block numbers (any may even occur on a per-transaction basis), so using block numbers for timing results in inconsistencies, especially when voting is involved across multiple chains. As of version 4.9, OpenZeppelin has modified their governor code to use a clock rather than block numbers, to avoid these sorts of issues.Proof of Concept
There are multiple instances of this issue as can be seen using this search command, i.e "implementations of
block.number
.Recommended Mitigation Steps
Implement different Clocks, and use the right one on each EVM
Assessed type
Other