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.

Sign up for GitHub

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

Jump to bottom

QA Report #241

Open
code423n4 opened this issue Feb 9, 2022 · 4 comments
Open

QA Report #241

code423n4 opened this issue Feb 9, 2022 · 4 comments
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L50

Vulnerability details

Impact

Block average time can change significantly, changing protocol parameters. If time dependence is desired, use block.timestamp. This can vary by as much as 15s (default Geth implementation), while block times can change measured time not by a constant, but scale it. Note that block time change WILL happen because of difficulty bomb and Ethereum 2.0.

Tools Used

Manual analysis

Recommended Mitigation Steps

Use block.timestamp for calculations where time is crucial.
@code423n4 code423n4 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 Feb 9, 2022
code423n4 added a commit that referenced this issue Feb 9, 2022
@code423n4
Czar102 issue #241
9d7c72e
@r2moon r2moon added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Feb 16, 2022
@r2moon
Copy link
Collaborator

r2moon commented Feb 16, 2022

reward will be accumulated per block

@GalloDaSballo
Copy link
Collaborator

I don't believe that in this case the severity of the finding is valid, at most a block will be delayed by a few seconds and over time difficulty is adjusted so that blocks are produced in a predictable way.

Given the circumstances I believe non-critical to be more appropriate as severity

@GalloDaSballo GalloDaSballo added 0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Apr 4, 2022
@JeeberC4
Copy link

Generating QA Report as warden had not submitting and judge downgraded issue. Preserving original title: Block average time

@JeeberC4 JeeberC4 added QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation labels Apr 21, 2022
@JeeberC4 JeeberC4 changed the title Block average time QA Report Apr 21, 2022
This was referenced Apr 21, 2022
Closed
Closed
Closed
@GalloDaSballo
Copy link
Collaborator

1+++

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@GalloDaSballo @r2moon @code423n4 @JeeberC4