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 #424

Open
c4-submissions opened this issue Oct 20, 2023 · 6 comments
Open

QA Report #424

c4-submissions opened this issue Oct 20, 2023 · 6 comments
Labels
bug Something isn't working grade-a Q-03 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sufficient quality report This report is of sufficient quality

Comments

@c4-submissions
Copy link
Contributor

See the markdown file with the details of this report here.
@c4-submissions c4-submissions added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Oct 20, 2023
c4-submissions added a commit that referenced this issue Oct 20, 2023
@c4-submissions
immeas issue #424
436c539
c4-submissions added a commit that referenced this issue Oct 20, 2023
@c4-submissions
immeas data for issue #424
61931e6
@c4-pre-sort
Copy link

raymondfam marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Oct 22, 2023
@alex-ppg
Copy link

Decent report with well-elaborated findings. L-01 may be a design decision (i.e. a critical update is needed due to a vulnerability and thus SubAccount implementations are forced to update).

@c4-judge
Copy link

alex-ppg marked the issue as grade-a

@0xad1onchain
Copy link

Thanks for the report. I agree with all Non Critical findings
L-01: SafeModerator and ConsoleFallbackHandler can only be updated by governance and maybe done in extreme scenarios. Only registries are kept immutable because they hold state
L-02: While I totally understand your point that attacker deployed the same bytecode at the deterministic CREATE2 address, for some reason safe changed their safe deployer implementation and removed the initializer as a part of address determination, leading us to question if there is a possible exploit we chose to keep it safe and make sure we use an address that we deploy

@c4-sponsor c4-sponsor added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Oct 27, 2023
@c4-sponsor
Copy link

0xad1onchain (sponsor) acknowledged

@c4-judge
Copy link

alex-ppg marked the issue as selected for report

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Oct 31, 2023
@C4-Staff C4-Staff added the Q-03 label Nov 6, 2023
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 grade-a Q-03 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sufficient quality report This report is of sufficient quality
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@0xad1onchain @alex-ppg @c4-judge @c4-sponsor @c4-pre-sort @C4-Staff @c4-submissions