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

bad_build_check didn't catch latest Jazzer breakage #8276

Open
oliverchang opened this issue Aug 17, 2022 · 11 comments · May be fixed by #8341
Open

bad_build_check didn't catch latest Jazzer breakage #8276

oliverchang opened this issue Aug 17, 2022 · 11 comments · May be fixed by #8341

Comments

@oliverchang
Copy link
Collaborator

Context: #8275

bad_build_check should've caught this and prevented builds from getting uploaded.

@jonathanmetzman

@oliverchang
Copy link
Collaborator Author

CC @fmeum too

@jonathanmetzman
Copy link
Contributor

Related: #8241

@fmeum
Copy link
Contributor

fmeum commented Aug 17, 2022

The Jazzer bug caused a crash if the fuzz target wasn't executed with a -seed argument. Both the bad build check and all our internal tests did that for the sake of reproducibility.

Not sure what to do about this particular bug type - we now have an internal one covering it.

@oliverchang
Copy link
Collaborator Author

Could we perhaps add one extra check (for libFuzzer engines) where we:

  • Try running with -runs=1
  • Try reproducing with an empty input

None of these should result in a crash if it's a good build.

@jonathanmetzman
Copy link
Contributor

Could we perhaps add one extra check (for libFuzzer engines) where we:

  • Try running with -runs=1
  • Try reproducing with an empty input

None of these should result in a crash if it's a good build.

IMO the check worked as it should. There was nothing wrong with Jazzer projects and reporting the bugs there instead of when we update Jazzer is not great UX. Ideally we wcould make seperate tests that run during updating

@oliverchang
Copy link
Collaborator Author

Could we perhaps add one extra check (for libFuzzer engines) where we:

  • Try running with -runs=1
  • Try reproducing with an empty input

None of these should result in a crash if it's a good build.

IMO the check worked as it should. There was nothing wrong with Jazzer projects and reporting the bugs there instead of when we update Jazzer is not great UX. Ideally we wcould make seperate tests that run during updating

If we don't pin, yes.

If we had these bad build checks and we pin, then we can catch them with our trial_build checks right?

Even ignoring Jazzer, making sure a target doesn't crash with an empty input is pretty valid right?

@jonathanmetzman
Copy link
Contributor

Right, I think we should absolutely pin though

@jonathanmetzman
Copy link
Contributor

Even ignoring Jazzer, making sure a target doesn't crash with an empty input is pretty valid right?

I guess. I think our existing bad build check probably cover this 99% of the time and just happened to miss it here.

@oliverchang
Copy link
Collaborator Author

Indeed. Let's:

  1. Keep pinning Jazzer
  2. Add a test to make sure that running against with an empty input does not crash in some way to bad_build_check.

@fmeum you mentioned being able to help with this. Would you be able to help us with a PR here? Thanks!

@fmeum
Copy link
Contributor

fmeum commented Aug 20, 2022

@oliverchang With empty input, do you mean an invocation of the form fuzz_target empty_file? If so, that would have caught the issue and I can add it in a PR.

@oliverchang
Copy link
Collaborator Author

@oliverchang With empty input, do you mean an invocation of the form fuzz_target empty_file? If so, that would have caught the issue and I can add it in a PR.

Yep!

fmeum added a commit to CodeIntelligenceTesting/oss-fuzz that referenced this issue Aug 24, 2022
Adds a new bad build check for libfuzzer targets that runs the fuzzer on
an empty file without any further arguments. This is specifically meant
to catch issues with argument parsing in libFuzzer derivates such as
Jazzer.

Fixes google#8276
fmeum added a commit to CodeIntelligenceTesting/oss-fuzz that referenced this issue Sep 13, 2022
Adds a new bad build check for libfuzzer targets that runs the fuzzer on
an empty file without any further arguments. This is specifically meant
to catch issues with argument parsing in libFuzzer derivates such as
Jazzer.

Fixes google#8276
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants