-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Golang internal library fuzzers #2188
Conversation
exciting! I wish we could do it the right way (golang/go#19109) but this way is better than none. Expect lots of things to not work. E.g. the coverage dashboard.
(obviously, because you are using libFuzzer's You will need a project.yaml Is this expected to work with AFL? You probably don't need AddressSanitizer, but there is currently no way to not have the asan build. I would appreciate if you could add a short readme to https://github.com/guidovranken/libfuzzer-go @dvyukov FYI |
projects/golang/build.sh
Outdated
@@ -0,0 +1,54 @@ | |||
# Get golang | |||
wget https://dl.google.com/go/go1.11.1.linux-amd64.tar.gz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should go to the Docker file.
Also, why fixed revision and not the head?
projects/golang/build.sh
Outdated
|
||
mkdir -p $GOPATH/src/github.com/dvyukov/ | ||
cd $GOPATH/src/github.com/dvyukov/ | ||
git clone https://github.com/dvyukov/go-fuzz-corpus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here and below: all download should go to Dockerfile
projects/golang/Dockerfile
Outdated
@@ -0,0 +1,21 @@ | |||
# Copyright 2016 Google Inc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix the year.
projects/golang/build.sh
Outdated
fuzzer=$(basename $1) | ||
echo "Fuzzer is $fuzzer" | ||
$CC $CFLAGS -Wall -Wextra $GOPATH/src/github.com/guidovranken/libfuzzer-go/C/main_libFuzzer_extra_counters.c -g -O3 -c -o main.o | ||
./go-fuzz-build -o $fuzzer.a github.com/dvyukov/go-fuzz-corpus/$fuzzer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add some comments on how the code gets instrumented here.
I assume this is source-to-source thing that go-fuzz uses, but not sure, and other readers won't know.
Just an example, here is how a fuzzing session with a bug report will look like in this project:
|
one more.
|
The only way I can think of to improve it is to generate more verbose panics in the fuzzer, eg here: https://github.com/dvyukov/go-fuzz-corpus/blob/master/url/main.go#L24 In the stack trace, I'm working on addressing your remarks now. |
projects/golang/Dockerfile
Outdated
MAINTAINER guidovranken@gmail.com | ||
|
||
RUN apt-get update && apt-get install -y build-essential wget | ||
RUN wget https://dl.google.com/go/go1.11.1.linux-amd64.tar.gz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of go-fuzz-corpus tests the standard library, so if we download the fixed revision, we test the fixed revision.
I would assume that all existing fuzzers should checkout a git with the project to get latest sources for testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And FWIW 1.12 is already out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason I get an import cycle error when I run it on 1.12. Any idea why? To test s/1.11.1/1.12/g in Dockerfile and build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What import cycle? On what binary? What is "build"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
failed to execute go build: exit status 1
import cycle not allowed
package github.com/dvyukov/go-fuzz-corpus/url/go.fuzz.main
imports github.com/dvyukov/go-fuzz-corpus/url
imports fmt
imports go-fuzz-dep
imports syscall
imports runtime
imports runtime/internal/math
imports go-fuzz-dep
Happens for 1.12 but not for 1.11.1.
To test:
git clone https://github.com/google/oss-fuzz
cd oss-fuzz
[change downloaded Go version from 1.11.1 to 1.12 in projects/golang/Dockerfile]
python infra/helper.py build_fuzzers golang
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding runtime/internal/math here should help:
https://github.com/dvyukov/go-fuzz/blob/master/go-fuzz-build/main.go#L302-L304
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That appears to fix it, thanks!
With regards to using the latest Go version, does the project offer nightly binaries? If not I'll look into https://golang.org/doc/install/source and try if I get build the latest version from the repo (I've never done that before).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind sending PR for go-fuzz with the runtime/internal/math change?
Go does not offer nightly releases, but it offer simple and working build :)
git clone; cd go/src; ./make.bash
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m actively updating Go-fuzz-build to work with go/packages, which should fix this issue and similar ones in the future. I hope to send a PR in the next few days. I’d love help testing and reviewing it—it ends up being a pretty major rewrite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind sending PR for go-fuzz with the runtime/internal/math change?
dvyukov/go-fuzz#211 should fix this, in a much deeper way. Still needs testing (and probably fixes) on Windows. Help there--even just bug reports--would be deeply appreciated.
Reviews from any/all are also very welcome. It's a large change.
This is awesome! |
I propose we review your fuzzers and raise the threshold for panicking where applicable (eg. should For my other Go fuzzer at oss-fuzz (bignum-fuzzer), security@golang.org is in the bug report CC. |
I guess asking them is the right option. There can be lots of bugs, so they should be prepared. Maybe they will say to open all them (easily discoverable by anybody anyway). |
IMHO go-fuzz-corpus is in need of some attention before it can be unleashed in an automated system. (1) There are existing false positives. Off the top of my head, the json fuzz function incorrectly reports round trip failures when the fuzz data contains duplicate keys. And the gofmt fuzz has false positives around \r and comments. You’ll find these quickly enough if you start fuzzing. :) But I think they should be fixed before turning it on—there has been resistance to fuzzer bugs in the past, and it’d be better to start on a good footing. (2) The fuzz functions contain a bunch of exceptions for bugs that have been fixed, particularly the go/types one. Some other things while I’m here, and apologies for drifting off topic. (1) Naive question: will new generate corpus entries be upstreamed somewhere discoverable? I’m working on improving go-fuzz, and I’d love to have a throughly saturated corpus to try to improve. (2) If I want to request oss-fuzz add support for starlark, how do I do that? I have a decently developed fuzz function and a pretty large corpus already. (3) How do I get (moderately) more involved here? I found this only by accident, backlinked from the Go repo issue you filed. |
Agree. It can make sense to start with just 1, safe fuzzer (fmt?) to get everything else working. And then add other fuzzers one-by-one or in batches. |
I would love for the It looks like the easy way to do that would be to have @dvyukov @josharian How do you feel about go-fuzz-corpus slowly migrating into the standard library? Is it too soon/not something you want to happen? |
I'd love fuzzing to be officially supported in the toolchain. There are important unanswered questions, though, among them: Do you check in the corpus entries? If so, where do they live? Does cmd/go need to be aware of them? If so, can we do that independently of fully integrating fuzzing into the toolchain? These seem pretty apropos for moving go-fuzz-corpus into the main repo. |
I think that would be great! |
But we don't burn any bridges by committing Fuzz functions into std lib, so we don't have to answer these questions right now. |
Then where does the generated corpus live? |
Somewhere in OSS-Fuzz. |
I dunno. Seems awkward to document to me, which is not usually a good sign. But I'm just one guy. :) |
We don't document it. The Fuzz function can be used without corpus by anybody. Corpus is not checked-in. If you want it, run it locally and collect the corpus. Even if we decide on the format, I am not sure OSS-Fuzz will be able to follow it (both format and permissions to check-in). So it looks mostly orthogonal to me. We can check in Fuzz functions, use them on OSS-Fuzz, and in parallel start figuring out how we want to store corpus for manual runs. Once we figure out the first option to try, we can manually check-in the corpus according to this scheme and see how well it works. But OSS-Fuzz will work with own corpus meanwhile. I wrote down my thoughts re corpus location in the proposal doc: |
I say we start by moving reliable As a start, the corpus can be in OSS-Fuzz and the reports can go to my inbox. Then we iterate to find better answers to those questions and to make the reports more actionable, the inputs more flexible, the build more performant, etc. :) |
Sounds good to me! |
To be clear, @dvyukov did all the hard work in terms of instrumentation and library fuzzers. For personal purposes, I only hacked his instrumentation tool for direct usage with libFuzzer. Seeing that it was technically possible to fuzz Go libraries with oss-fuzz, I created this PR. That said, my understanding of Go intricacies and efforts to implement built-in Go fuzzing report are limited, but I will try to read up on it. As said, the PR as it stands uses https://github.com/guidovranken/libfuzzer-go, which is my fork/hack of @dvyukov https://github.com/dvyukov/go-fuzz. In short, it acts as a conduit between Another feature that is preferable to have is libFuzzer's value profile/autodict support. In short, it would be nice that if a comparison is performed in Go (eg
The up-to-date corpus is accessible through https://oss-fuzz.com/ (once the project is online). It can be manually be pushed to a public repo. See for example OpenSSL, which frequently does this: https://github.com/openssl/openssl/commits/master/fuzz/corpora
To build a libFuzzer-backed Go fuzzer:
For oss-fuzz integration, see Dockerfile + build.sh in this PR. One more thought: strong seed corpora can be generated from existing public corpora (like that of OpenSSL) for generic formats like ASN1, PEM, JSON, X509, etc. |
What exactly are talking about? Where is this implemented in your fork? |
It sounds reasonable for libfuzzer to provide some callback for this along with __libfuzzer_extra_counters to make it possible to fully replace instrumentation. In your current implementation I see some hooks for this, but it is not glued to libfuzzer, right? I see that memcmpCBPtr is always set to NULL. |
I see there are at least 3 SSL implementations tests, is there a way to share common input formats on OSS-Fuzz? |
So should I add golang-fuzz@googlegroups.com to project.yaml or is the intention to manually forward reports there? And should I add |
Please add golang-fuzz@googlegroups.com directly, and |
It means that all bugs are immediately publicly disclosed at https://bugs.chromium.org/p/oss-fuzz/issues/list |
Alright, the stack parsing PR is almost landed. One last thing to mention here before landing this as well. We should be able to suppress libFuzzer's stacktrace from the log by using
But I don't think it's too big of a deal. The go frames will be parsed first and used in the crash state anyways. Plus, in case of some really weird and unexpected crash, libFuzzer logs may appear to be more useful compared to the standard If you would like to get rid of these frames, we can add
but I think it's fine for now, we can do it later. |
FYI, we have to use ASan, otherwise corpus pruning won't work. I'm doing some speed measurements locally to decide whether we'll proceed with ASan only or with both ASan and UBSan (in case UBSan works much faster). |
Don't observe any significant performance difference, likely because there isn't much code being instrumented by either of the sanitizers. I'll merge this and update the project.yaml myself right after that. Bad build check succeeded locally. CF deployment is in the progress, btw. |
Good work, everyone! |
They should appear at: This seems to be the query to find them: |
The group is set up like all |
FYI, we've improved stack parsing for Go crashes on ClusterFuzz side, and a couple of previously reported issues may be reported again with another (better) crash type. Could update the existing crashes manually, but didn't bother as there are not too many being affected and in general it's a one-off change as we've just started running Go fuzzers. |
Is there a doc listing the steps for getting more fuzzing functions running by default? Is this something we could effectively crowd-source? |
I'll document OSS-Fuzz side of things later today after landing and testing #2735. |
Do you mean stdlib fuzzing, or fuzzing Go software generally? As for the stdlib, only a fraction of https://github.com/dvyukov/go-fuzz-corpus is currently running on OSS-Fuzz. Adding more would simply consist of adding a One caveat is that it shouldn't crash on the seed corpus. |
I was specifically thinking of stdlib fuzzing. Good to know adding more fuzz functions are easy. So it's just a question of getting new fuzz functions into go-fuzz-corpus. |
Update: looks like updating the OSS-Fuzz documentation may take a bit longer. So, for the time being:
|
How can I add @odeke-em as a registered maintainer for these fuzzers? |
The metadata is here: |
yes @odeke-em in the auto_ccs field, docs here - https://google.github.io/oss-fuzz/getting-started/new-project-guide/#primary |
Thank you @josharian @dvyukov and @inferno-chromium! I've mailed this PR #5189 adding myself as a maintainer. |
For a context of the error I got, I tried to go download the testcase for https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=30042&q=label%3AProj-golang and instead got this and for a couple of other bugs. My goal is to get them filed for the Go project and to fix them too. |
This plugs the fuzzers in https://github.com/dvyukov/go-fuzz-corpus into libFuzzer.
Is there any interest in this?