Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hardcoded block numbers will lead to unsuccessful off-boarding #1034

Open
c4-bot-3 opened this issue Dec 28, 2023 · 7 comments
Open

Hardcoded block numbers will lead to unsuccessful off-boarding #1034

c4-bot-3 opened this issue Dec 28, 2023 · 7 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-816 grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality

Comments

@c4-bot-3
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/LendingTermOffboarding.sol#L36

Vulnerability details

Impact

Off-boarding is with a duration of 46523 blocks (7 days), calculated from block.number on the Ethereum chain 13 sec/block, but the development team wants to deploy in L2 chains as well:

Deployment: we anticipate to launch on Ethereum mainnet & L2s like Arbitrum.

There is no way to change the off-boarding duration and most of the proposals on the L2 chains will fail due to the high amount of votes required in a short amount of period.

Proof of Concept

https://github.com/0xJuancito/multichain-auditor?tab=readme-ov-file#block-production-may-not-be-constant
The most widely used L2 chains are Arbitrum, Optimism, and Polygon zkEVM. But the average block time is different on all of them:

  • Arbitrum - 0.26 sec
  • Optimism - 2 sec
  • Polygon zkEVM - 7 sec

On Optimism 46523 blocks will pass for approximately 26 hours as there are 1800 blocks per hour.

If we take Arbitrum 46523 blocks will pass for approximately 3 hours and 36 minutes as there are 13846 blocks per hour.

We can see from the proposals (GIP_0.sol) 10_000_000e18 set as an OFFBOARD_QUORUM and we can conclude that there is no way quorum to be satisfied within 216 minutes.

Governor can change the quorum for a given LendingTermOffboarding.sol contract but this exposes risk as this will significantly decrease the decentralization.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider allowing the deployer to set the POLL_DURATION_BLOCKS from the constructor instead of assigning it to constant.

Assessed type

Math

@c4-bot-3 c4-bot-3 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 28, 2023
c4-bot-10 added a commit that referenced this issue Dec 28, 2023
@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Dec 29, 2023
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as sufficient quality report

@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as duplicate of #1131

@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as duplicate of #1012

@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as duplicate of #816

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Jan 24, 2024
@c4-judge
Copy link
Contributor

Trumpero changed the severity to QA (Quality Assurance)

@c4-judge
Copy link
Contributor

Trumpero marked the issue as grade-b

@c4-judge
Copy link
Contributor

Trumpero marked the issue as grade-a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-816 grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants