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

cmd/go: fuzz crash minimizer should try deleting, rewriting input bytes #48129

Open
rsc opened this issue Sep 2, 2021 · 2 comments
Open

cmd/go: fuzz crash minimizer should try deleting, rewriting input bytes #48129

rsc opened this issue Sep 2, 2021 · 2 comments

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Sep 2, 2021

The fuzzer found a crash in a package I'm working on:

go test fuzz v1
string("--\xd6M\x8b<!---->")

I can manually minimize this input further by just doing a trial deletion of each byte, keeping each deletion only if it preserves the crash (not the exact panic message, because slice bounds are changing while remaining invalid, but the fact of a crash). This reduces the input to:

string("M\x8b<!---->")

Then I can change each input byte to an A, one at a time, keeping the crashes. This simplifies to:

string("AA<!---->")

which appears to be the simplest possible (and more readable) form of this crasher.

What I did, the crash minimizer should be able to do. It will result in more ASCII-only crashes. I would suggest for the input byte rewriting to have a priority list ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789 !"#$%&'()*+,-./:;<=>?@[\]^_`{|}~ trying letters, digit, space, and finally punctuation for each byte, stopping at the first one that preserves the crash. The letters could be thinned out and the punctuation elided if this is too much. Even ABCXYZabcxyz012789 would be fine.

/cc @jayconrod @katiehockman

@rsc rsc added this to the Go1.18 milestone Sep 2, 2021
@rsc
Copy link
Contributor Author

@rsc rsc commented Sep 2, 2021

Another good possibility for simplifcation would be seeing if setting x[i] = x[i-1] preserves the crash. The minimizer did a good job cutting "&#XabD02caD0cabD;6206b&#Xca0cabD06b&#XD00cabD06\n" down to "&#XaD0cabD;", but then it could have tried that rule and ended at "&#Xaaaaaaa;".

@jayconrod jayconrod added the fuzz label Sep 9, 2021
@katiehockman
Copy link
Member

@katiehockman katiehockman commented Sep 14, 2021

I'm going to remove the release-blocker label for this one. @rsc LMK if you disagree strongly.

This sounds like an improvement to the minimizer, rather than a bug. Although I definitely agree this is an improvement worth doing, I am trying to trim down the release-blocking work as much as possible and limit it only to features that are essential for the UX and for bugs.
(fwiw we may very well still get to this before 1.18)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants