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

Fuzz testing #2

Closed
acln0 opened this issue Jul 13, 2018 · 5 comments
Closed

Fuzz testing #2

acln0 opened this issue Jul 13, 2018 · 5 comments
Assignees
Milestone

Comments

@acln0
Copy link
Member

acln0 commented Jul 13, 2018

Since this package touches untrusted user input, it would be a good idea to fuzz FromString, (*UUID).UnmarshalText and similar.

Assuming we use https://github.com/dvyukov/go-fuzz, what should we do about the fuzz testing corpus? I think there are three options:

  • keep it in the repository, under testdata/{corpus,crashers,suppressions}: this clutters things quite a bit, unfortunately
  • use a separate repository, perhaps gofrs/uuid-test-corpus: this may be overkill
  • don't commit the corpus at all: the parser is not very large, and is unlikely to change very much, so perhaps this is the simplest thing to do

Thoughts?

@theckman
Copy link
Member

@acln0 how large is this file? I may be willing to host it somewhere like Google Cloud Storage, since it should only cost a few cents a month I'd think.

@acln0
Copy link
Member Author

acln0 commented Jul 13, 2018

@theckman The fuzz test corpus contains multiple files, one for every interesting input the fuzzer has found. The general notion is that the developer seeds the corpus with interesting inputs (usually based on test cases), by creating one file per input and storing it in the work directory, under corpus/. Then, the fuzzer takes over, and mutates the corpus to find more interesting inputs. Complex parsers often commit the corpus so fuzzing can resume from the previous state, which usually covers a diverse set of inputs.

Given that the parser doesn't have many branches and the UnmarshalText code is unlikely to change very much in the future, I think we should offer a flag to place test inputs into the corpus directory as individual files, and instructions on how to run the fuzzer if anyone needs to, in the future.

@theckman
Copy link
Member

Sure. I wasn't sure how much data we'd think we'd grow to, in the context of not making the repository unnecessarily large. If it was going to be too large, I was thinking of offering the seeded+mutated corpus as a publicly downloadable archive. That way people could use it for local testing with a consistent set of data. If it's not going to be too large, then we could just check the files in.

I've not ran go-fuzz over anything to get a rough idea of how thorough it would be at generating more data.

@acln0
Copy link
Member Author

acln0 commented Jul 13, 2018

For this particular case, it's quite small. I ran the fuzzer on this for ~30 minutes, and it amounted to somewhere between 20 and 30 files. If you're okay with checking them in, I think they should live under testdata/.

@theckman
Copy link
Member

That sounds perfectly acceptable. 👍

@acln0 acln0 self-assigned this Jul 14, 2018
@acln0 acln0 mentioned this issue Jul 15, 2018
@theckman theckman added this to the 1.3.0 milestone Jul 15, 2018
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

No branches or pull requests

2 participants