Add support to fuzz from go-fuzz input by posener · Pull Request #35 · google/gofuzz

posener · 2020-01-23T21:19:45Z

Add a helper function that enables using gofuzz (this project) with go-fuzz for continuose fuzzing. Essentially, it enables translating the fuzzing bytes from go-fuzz to any Go object using this library.

This change will enable using this project with fuzzing websites such as fuzzit.dev or fuzzbuzz.io.

The underlying implementation was an idea of @lavalamp, by which a random source is created that generates random numbers from the input bytes slice. In this way changes in the source result in logical changes in the random data, which enables the fuzzer to efficiently search the space of inputs.

Fixes #33

posener · 2020-01-23T21:31:53Z

Seems like the build failure has nothing to do with my code.

lavalamp · 2020-01-23T21:59:02Z

I'll have to look into the test failure

posener · 2020-01-24T06:40:51Z

@lavalamp Thanks for the help in this one.
Please re-review and see if you find the code and style as you like them.
I also updated the commit message and PR message.

BTW, I'll be off the grid in the upcoming week.

lavalamp

This looks great-- my comments are mostly cosmetic.

Thanks!

(p.s. can you edit the commit messages to e.g. not include the @ in front of lavalamp? github sends people email every time anything is done with such commits :) )

posener · 2020-01-25T10:57:25Z

A few more things:

What do you think about the name of the exported function?
What do you think about adding something to the README?

lavalamp

I can only find 2 tiny nits, sorry for the delay, I got busy.

I'm willing to take this as is, but I was thinking and I think we can improve on it a bit, and since we won't be able to change this after the fact except by introducing a new version of it, I'll say what I'm thinking and you can decide if it's worth changing.

The current version lets the fuzzer lock in the values of the first N random calls, but changing the length of the byte slice completely changes the random sequence for the rest of the calls, in two ways:

Since the seed is padded, changing the length changes how much padding is needed, meaning only insertions/removals of 8 bytes keep the same sequence
...but an insertion (removal) of 8 bytes will delay (hasten) the first call to the fallback generator, meaning that although the sequence is the same, it's being called for different locations, which will change every site.

Here's an idea that solves these problems:

Use the first 8 bytes as a seed.
When returning a Uint64 directly from the data, also get a random Uint64 from the fallback source and throw it away.

I think this makes length changes much less consequential, which should let the fuzzer search with them much more effectively?

lavalamp · 2020-02-03T17:27:01Z

oh, and the questions:

Yes, a readme change is welcome
The name is OK, you could also consider making the data-backed random source public, move the comment there, and just let people chain it together with the RandSource() method. That would also let people reuse this for other purposes, I don't have anything in mind but it feels like the sort of thing that might be useful in general.

posener · 2020-02-04T17:26:54Z

Thanks!
Not sure about exposing the struct. I thought that exposing the bare functionality through a function might be easier to use. It can always be exposed later when needed I guess.

lavalamp

Final round of nits and it looks like you'll need to rebase, too, sorry...

lavalamp · 2020-02-11T00:28:49Z

+// New returns a new ByteSource from a given slice of bytes.
+func New(input []byte) *ByteSource {
+	if len(input) == 0 {
+		panic("ByteSource was initiated with empty input")


hm, mention this in the function comment, or (my preference) use a seed of 0 in this case?

posener · 2020-02-11T20:56:23Z

Before commiting this, I would like you to take a look at https://github.com/posener/fuzzing.
There, I avoided going through the rand library (except for the fallback that we run out of input bytes), and just decode the bytes to Go types.
I think that approach has slight advantages, regarding the fuzzer exploration, and additionally, it is a bit simpler. Maybe you have ideas how to use that method to work with gofuzz?

Add a helper function that enables using gofuzz (this project) with go-fuzz (https://github.com/dvyukov/go-fuzz) for continuose fuzzing. Essentially, it enables translating the fuzzing bytes from go-fuzz to any Go object using this library. This change will enable using this project with fuzzing websites such as fuzzit.dev or fuzzbuzz.io. The underlying implementation was an idea of lavalamp, by which a random source is created that generates random numbers from the input bytes slice. In this way changes in the source result in logical changes in the random data, which enables the fuzzer to efficiently search the space of inputs. Fixes google#33

lavalamp · 2020-02-19T17:44:25Z

Sorry I've been super busy and that was a complicated question :)

Do you have a specific spot in that repo I can look at? I don't think I found what you were talking about in a quick skim...

posener · 2020-02-20T16:45:46Z

It is a small repo, you can check the fuzz.go. The technique is similar and simple: create a random seed from the first 8 bytes. Then read an amount of required bytes according to the requested type and use the binary.BigEndian to decode them.

lavalamp · 2020-03-09T20:37:25Z

Ah, I understand now. Yes, I agree, that is nicer. I'm not sure of a way to do it without defining an interface that covers rand.Rand?

(sorry for the delay; busy few weeks!)

posener · 2020-03-11T09:30:58Z

Maybe it is better just to close this one.
I'm not sure about it anymore.
Please do so if you agree.

lavalamp · 2020-03-16T17:04:17Z

+func (s *ByteSource) consumeUint64() uint64 {
+	var bytes [8]byte
+	_, err := s.Read(bytes[:])
+	if err != nil {


I think we need a test with data that's a length not divisible by 8-- this might return an EOF at some point?

This is OK. https://play.golang.org/p/9uC8M9Nm5pE
But according to the io.Reader interface definition, you are right, the implementation can return io.EOF even when there were bytes that were read.

It is however tested, when the input bytes are the numbers 1..9.

Ah, I do see that test now, sorry!

lavalamp · 2020-03-16T17:04:57Z

After more thought, I think this is still useful. Thanks for the idea! I did see one more thing that should probably be addressed though, if you still want to pursue this.

posener · 2020-03-17T19:55:03Z

Yes sure, lets continue with it. Why not

lavalamp · 2020-03-17T19:59:01Z

Thanks!

posener · 2020-03-17T20:12:47Z

Thank for the review! It was a pleasure!

googlebot added the cla: yes label Jan 23, 2020

lavalamp reviewed Jan 23, 2020

View reviewed changes

Comment thread fuzz.go

lavalamp reviewed Jan 23, 2020

View reviewed changes

Comment thread fuzz.go Outdated

lavalamp reviewed Jan 23, 2020

View reviewed changes

Comment thread fuzz.go Outdated

posener force-pushed the gofuzz branch 6 times, most recently from 9f1dad6 to 019391e Compare January 24, 2020 06:35

posener changed the title ~~Add NewFromGoFuzz~~ Add support to fuzz from go-fuzz input Jan 24, 2020

lavalamp reviewed Jan 24, 2020

View reviewed changes

Comment thread bytessource.go Outdated

Comment thread bytessource.go Outdated

Comment thread fuzz_test.go Outdated

Comment thread fuzz_test.go Outdated

Comment thread fuzz.go

Comment thread fuzz.go

posener force-pushed the gofuzz branch from 019391e to 8200926 Compare January 25, 2020 10:56

lavalamp reviewed Feb 3, 2020

View reviewed changes

Comment thread bytessource.go Outdated

Comment thread bytessource.go Outdated

posener force-pushed the gofuzz branch 2 times, most recently from a9a49e9 to e11177b Compare February 8, 2020 11:06

lavalamp reviewed Feb 11, 2020

View reviewed changes

posener force-pushed the gofuzz branch from 2aa4d6e to f763a7d Compare February 14, 2020 18:49

lavalamp reviewed Mar 16, 2020

View reviewed changes

consumeUint64: don't panic on EOF

c0fd83d

lavalamp merged commit c89cefb into google:master Mar 17, 2020

posener deleted the gofuzz branch March 17, 2020 20:12

Conversation

posener commented Jan 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

posener commented Jan 23, 2020

Uh oh!

Uh oh!

lavalamp commented Jan 23, 2020

Uh oh!

posener commented Jan 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lavalamp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

posener commented Jan 25, 2020

Uh oh!

lavalamp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lavalamp commented Feb 3, 2020

Uh oh!

posener commented Feb 4, 2020

Uh oh!

lavalamp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lavalamp Feb 11, 2020

Choose a reason for hiding this comment

Uh oh!

posener Feb 11, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

posener commented Feb 11, 2020

Uh oh!

lavalamp commented Feb 19, 2020

Uh oh!

posener commented Feb 20, 2020

Uh oh!

lavalamp commented Mar 9, 2020

Uh oh!

posener commented Mar 11, 2020

Uh oh!

lavalamp Mar 16, 2020

Choose a reason for hiding this comment

Uh oh!

posener Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

posener Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

lavalamp Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

lavalamp commented Mar 16, 2020

Uh oh!

posener commented Mar 17, 2020

Uh oh!

lavalamp commented Mar 17, 2020

Uh oh!

posener commented Mar 17, 2020

Uh oh!

Reviewers

posener commented Jan 23, 2020 •

edited

Loading

posener commented Jan 24, 2020 •

edited

Loading