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

x/image: add a sample fuzz test for prototype of "fuzzing as a first class citizen" #30719

Closed
thepudds opened this issue Mar 10, 2019 · 8 comments
Closed

x/image: add a sample fuzz test for prototype of "fuzzing as a first class citizen" #30719

thepudds opened this issue Mar 10, 2019 · 8 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@thepudds
Copy link
Contributor

Summary

This is an issue covering adding a sample Fuzz function to x/image in support of the proposal to "make fuzzing a first class citizen" in #19109.

The suggestion in this issue is to add https://github.com/dvyukov/go-fuzz-corpus/blob/master/tiff/tiff.go to golang.org/x/image/tiff, perhaps as tiff_fuzz.go or fuzz.go. (Ultimately, the proposal is allow Fuzz functions to reside in a standard *_test.go file, but currently I think dvyukov/go-fuzz does not support that).

It likely should be protected by a gofuzz build tag (or alternatively, a fuzz build tag; some additional discussion below).

Once a single Fuzz function is added, additional Fuzz functions could be added to golang.org/x/image or other golang.org/x repositories, but that could be viewed as a follow-on steps for now.

If there is agreement to proceed on this issue, likely someone from the broader Go community could send a CL.

Background

I suspect the people interested in weighing in on this issue here might not wish to re-read the entirety of the now lengthy #19109 discussion, so a brief recap:

After some discussion in #19109, the core Go team asked for a prototype before evaluating the proposal. For example, comments from Russ in #19109 (comment), #19109 (comment), and #19109 (comment), including:

it still seems like the right next step is to make 'go-fuzz' the separate command as close to 'go fuzz' the proposed standard command as possible, and to add fuzz tests to at least the x subrepos and maybe the standard library, so that we can understand the implications of having them in the source repos (including how much space we're going to spend on testdata/fuzz corpus directories).

In order to break things down into more manageable chunks of work (including in an attempt to break out items that must be done by someone on the core Go team vs. could be done by someone from the broader community), the following steps were suggested in #19109 (comment):

Draft Goals for a Prototype

To be done before an evaluation can be made by the Go proposal review committee:

  1. Prototype proposed CLI, including interaction with existing 'go test'.
  2. Add some sample fuzz tests to at least the x subrepos and maybe the standard library.
  3. Start an initial set of corpus directories for the x repos and maybe the standard library (for example, earlier, the proposal suggested "For the standard library it is proposed to check in corpus into golang.org/x/fuzz repo").
  4. Understand how much space is used in corpus directories for x subrepos and/or standard library based on those sample fuzz tests.
  5. Add a new fuzzing signature (or change the existing Fuzz(data []byte) int signature) to work with testing.TB.

Some progress on Step 1 is thepudds/fzgo, a simple work-in-progress prototype that currently implements the majority of the proposed command from the March 2017 proposal document.

Separately, @josharian has done some experimentation regarding compiler level integration, including some related discussion in #29430.

Additional Details

This issue is intended to cover at least the start of Step 2 above. @dvyukov suggested in #19109 (comment) starting with x/image.

Regarding what build tag to use here, dvyukov/go-fuzz defaults to setting the gofuzz build tag:

go-fuzz-build builds the program with gofuzz build tag, this allows to put the
Fuzz function implementation directly into the tested package, but exclude it
from normal builds with // +build gofuzz directive.

The March 2017 proposal document suggests supporting the fuzz build tag. thepudds/fzgo sets both gofuzz and fuzz build tags. For the purposes of this issue, I suggest using the gofuzz build tag for now in x/image given that is the default for dvyukov/go-fuzz.

This issue is not currently intended to cover Step 3 above in terms of creating a new corpus repository under golang.org/x (which would require action by specific people on the core Go team). I would be happy to file a separate issue for creating a new corpus repository, or if people prefer this single issue could be expanded in scope to cover discussing that as well.

Comments? Concerns? Alternative thoughts on how to make concrete progress on Step 2 above?

CC @FiloSottile @bradfitz @acln0
@gopherbot gopherbot added this to the Unreleased milestone Mar 10, 2019
@thepudds thepudds changed the title x/image: add a sample fuzz test for prototype of "fuzzing a x/image: add a sample fuzz test for prototype of "fuzzing as a first class citizen" proposal Mar 10, 2019
@josharian
Copy link
Contributor

This was discussed also at google/oss-fuzz#2188 (comment) and I believe @FiloSottile is taking the lead on it.

FYI, I have been working on go-fuzz some recently, including improving the API (broadly interpreted). I have been doing so with one eye on #19109, but mainly with an emphasis on making it more useful and usable now. One of the next things I plan to do is your step 5 above ("Add a new fuzzing signature").

@dvyukov
Copy link
Member

dvyukov commented Mar 11, 2019

Sounds good to me.

I am pretty sure we do not want corpuses check-in with code. Based on my experience with go-fuzz[-corpus] this produces too much churn and changes. Imagine we have 100 fuzzers in the std lib, each day infra checks in corpus update for each. That's 100 additional changes each day and tons of diff on each check out and significant repo increase.
Maybe go modules can provide a nice solution to this? Corpus is in a separate repo but at the same time can be automatically discovered by go tool based on some convention?

@dvyukov dvyukov mentioned this issue Mar 12, 2019
Merged
@gopherbot
Copy link

Change https://golang.org/cl/167097 mentions this issue: tiff: add Fuzz function

@acln0
Copy link
Contributor

acln0 commented Mar 12, 2019

I also have an interest in fuzzing as a first class citizen, so I have sent a CL to hopefully make the conversation more concrete.

@thepudds
Copy link
Contributor Author

Earlier today, @dvyukov wrote in google/oss-fuzz#2188 (comment):

@thepudds proposed x/image, @FiloSottile proposed x/crypto at the first target. I don't think it matter much which one will be first. We can do both.

That makes sense to me. x/image might be a bit simpler, but personally I would be excited for any Fuzz function to land as a first example.

@thepudds
Copy link
Contributor Author

thepudds commented Mar 12, 2019 •
edited

@josharian Thank you for the reference to the conversation from the last 1-2 weeks in google/oss-fuzz#2188. I had not read that recent portion, but makes sense.

@FiloSottile Based on the comments from @dvyukov and the CL from @acln0, do you think it makes sense to proceed with this issue here?

@ianlancetaylor ianlancetaylor changed the title x/image: add a sample fuzz test for prototype of "fuzzing as a first class citizen" proposal x/image: add a sample fuzz test for prototype of "fuzzing as a first class citizen" Mar 20, 2019
@ianlancetaylor
Copy link
Contributor

CC @nigeltao

@nigeltao
Copy link
Contributor

General approach SGTM. I'm happy for people like @dvyukov or @josharian to review actual CLs.

@thepudds thepudds mentioned this issue Mar 21, 2019
Open
This was referenced Apr 2, 2019
Open
Closed
@golang golang locked and limited conversation to collaborators Apr 1, 2020
mrhyperbit23z0d added a commit to mrhyperbit23z0d/bhegde8 that referenced this issue Jun 6, 2022
@mrhyperbit23z0d
tiff: add Fuzz function
ced90e4 
This change adds a sample Fuzz test function to package tiff, under
the gofuzz build tag. The function is based on the tiff/tiff.go code,
from github.com/dvyukov/go-fuzz-corpus.

Fixes golang/go#30719
Updates golang/go#19109

Change-Id: I78771e9a1bd01651ba6ca421ba41f0c0e95d0c53
Reviewed-on: https://go-review.googlesource.com/c/image/+/167097
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: thepudds <thepudds1460@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
GalaxyForcew added a commit to GalaxyForcew/A1bisshy that referenced this issue Jun 6, 2022
@GalaxyForcew
tiff: add Fuzz function
fc22d9f 
This change adds a sample Fuzz test function to package tiff, under
the gofuzz build tag. The function is based on the tiff/tiff.go code,
from github.com/dvyukov/go-fuzz-corpus.

Fixes golang/go#30719
Updates golang/go#19109

Change-Id: I78771e9a1bd01651ba6ca421ba41f0c0e95d0c53
Reviewed-on: https://go-review.googlesource.com/c/image/+/167097
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: thepudds <thepudds1460@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
yi-ge3 added a commit to yi-ge3/wislie that referenced this issue Jun 6, 2022
@yi-ge3
tiff: add Fuzz function
44a0563 
This change adds a sample Fuzz test function to package tiff, under
the gofuzz build tag. The function is based on the tiff/tiff.go code,
from github.com/dvyukov/go-fuzz-corpus.

Fixes golang/go#30719
Updates golang/go#19109

Change-Id: I78771e9a1bd01651ba6ca421ba41f0c0e95d0c53
Reviewed-on: https://go-review.googlesource.com/c/image/+/167097
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: thepudds <thepudds1460@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
balloontmz6 added a commit to balloontmz6/Likewise42l that referenced this issue Jun 6, 2022
@balloontmz6
tiff: add Fuzz function
92fb984 
This change adds a sample Fuzz test function to package tiff, under
the gofuzz build tag. The function is based on the tiff/tiff.go code,
from github.com/dvyukov/go-fuzz-corpus.

Fixes golang/go#30719
Updates golang/go#19109

Change-Id: I78771e9a1bd01651ba6ca421ba41f0c0e95d0c53
Reviewed-on: https://go-review.googlesource.com/c/image/+/167097
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: thepudds <thepudds1460@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
snapbakkhfbav added a commit to snapbakkhfbav/SayedBaladohr that referenced this issue Oct 6, 2022
@snapbakkhfbav
tiff: add Fuzz function
163d0a6 
This change adds a sample Fuzz test function to package tiff, under
the gofuzz build tag. The function is based on the tiff/tiff.go code,
from github.com/dvyukov/go-fuzz-corpus.

Fixes golang/go#30719
Updates golang/go#19109

Change-Id: I78771e9a1bd01651ba6ca421ba41f0c0e95d0c53
Reviewed-on: https://go-review.googlesource.com/c/image/+/167097
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: thepudds <thepudds1460@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
MiderWong5ddop added a commit to MiderWong5ddop/sidie88f that referenced this issue Oct 7, 2022
@MiderWong5ddop
tiff: add Fuzz function
63df1e0 
This change adds a sample Fuzz test function to package tiff, under
the gofuzz build tag. The function is based on the tiff/tiff.go code,
from github.com/dvyukov/go-fuzz-corpus.

Fixes golang/go#30719
Updates golang/go#19109

Change-Id: I78771e9a1bd01651ba6ca421ba41f0c0e95d0c53
Reviewed-on: https://go-review.googlesource.com/c/image/+/167097
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: thepudds <thepudds1460@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
rorypeckwnt4v added a commit to rorypeckwnt4v/LearnByBhanuPrataph that referenced this issue Oct 7, 2022
@rorypeckwnt4v
tiff: add Fuzz function
9cc00ad 
This change adds a sample Fuzz test function to package tiff, under
the gofuzz build tag. The function is based on the tiff/tiff.go code,
from github.com/dvyukov/go-fuzz-corpus.

Fixes golang/go#30719
Updates golang/go#19109

Change-Id: I78771e9a1bd01651ba6ca421ba41f0c0e95d0c53
Reviewed-on: https://go-review.googlesource.com/c/image/+/167097
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: thepudds <thepudds1460@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
egorovcharenko9 added a commit to egorovcharenko9/RiceBIOC470z that referenced this issue Oct 7, 2022
@egorovcharenko9
tiff: add Fuzz function
99d055d 
This change adds a sample Fuzz test function to package tiff, under
the gofuzz build tag. The function is based on the tiff/tiff.go code,
from github.com/dvyukov/go-fuzz-corpus.

Fixes golang/go#30719
Updates golang/go#19109

Change-Id: I78771e9a1bd01651ba6ca421ba41f0c0e95d0c53
Reviewed-on: https://go-review.googlesource.com/c/image/+/167097
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: thepudds <thepudds1460@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
RafayGhafoorf added a commit to RafayGhafoorf/dustinsand8 that referenced this issue Oct 7, 2022
@RafayGhafoorf
tiff: add Fuzz function
f128e5e 
This change adds a sample Fuzz test function to package tiff, under
the gofuzz build tag. The function is based on the tiff/tiff.go code,
from github.com/dvyukov/go-fuzz-corpus.

Fixes golang/go#30719
Updates golang/go#19109

Change-Id: I78771e9a1bd01651ba6ca421ba41f0c0e95d0c53
Reviewed-on: https://go-review.googlesource.com/c/image/+/167097
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: thepudds <thepudds1460@gmail.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
7 participants
@josharian @dvyukov @ianlancetaylor @acln0 @nigeltao @gopherbot @thepudds