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

Integrate native go fuzzing #7055

Merged
merged 19 commits into from
Feb 8, 2022

Conversation

AdamKorcz
Copy link
Collaborator

@AdamKorcz AdamKorcz commented Dec 22, 2021

This PR is the tracker for native Go fuzz integration. It is expected to be ready when native fuzzing becomes available in Go 1.18.

The purpose of this PR is to enable Golang projects to run their Go-1.18 fuzzers in OSS-fuzz.

Design

From a high level, a native fuzz harness is rewritten to a go-fuzz-like harness which is then built with libfuzzer instrumentation by way of an extended version of the go114-fuzz-build script which is currently being used by OSS-fuzz.
This process is enabled by a custom implementation of *testing.F which has the purpose of generating the arguments to the fuzzer based on the byte array from libFuzzer and calling the fuzz function. In a native Go fuzz harness, any number of arguments can be passed to the fuzzer, and these can be of other types than a byte slice.
Each argument is generated with the go-fuzz-headers library which deterministically can generate all the supported types from a byte slice.

Testing

The integration has been tested on the following fuzzers:

Vitess

  • FuzzTabletManager_ExecuteFetchAsDba (src)
  • FuzzEqualsSQLNode (src)
  • FuzzIsDML (src)
  • FuzzNodeFormat (src)
  • FuzzNormalizer (src)
  • FuzzParser (src)
  • FuzzSplitStatementToPieces (src)

go-ethereum

  • FuzzRuntimeNative (src)
  • FuzzLesNative (src)
  • FuzzRlpNative (src)

istio

  • FuzzParseInputs (src)
  • FuzzKubeCRD (src)
  • FuzzParseMeshNetworks (src)
  • FuzzValidateMeshConfig (src)

As @DavidKorczynski and I work more on the integration, updates and documentation will be added to this PR.

Features

  • Does not require upstream changes to native Go fuzzing.
  • Does not require changes to OSS-fuzz runtime environment.
  • Works with OSS-fuzz's seed corpus and dictionaries.
  • Works with any number of parameters in the native harness.
  • Requires no functional changes to existing go fuzz coverage build. Some code will be reused and therefore reorganized. (Expected/pending)

Supported arg types:

  • []byte
  • string
  • int
  • int8
  • int16
  • int32
  • int64
  • uint
  • uint8
  • uint16
  • uint32
  • uint64
  • rune
  • float32
  • float64

All native supported arg types: https://github.com/golang/go/blob/master/src/testing/fuzz.go#L165. All of these will be supported by OSS-fuzz.

@AdamKorcz AdamKorcz force-pushed the native-go-fuzz-integration branch from 284e1dc to c9c77de Compare December 22, 2021 17:26
@AdamKorcz AdamKorcz force-pushed the native-go-fuzz-integration branch from 85e5e03 to 668f3d4 Compare January 7, 2022 13:39
@prestonvanloon
Copy link

@AdamKorcz is it possible to support fuzz targets with 2+ fuzz arguments?

image

@AdamKorcz
Copy link
Collaborator Author

AdamKorcz commented Jan 10, 2022

is it possible to support fuzz targets with 2+ fuzz arguments?

Yes. This is already supported at this stage.

@AdamKorcz AdamKorcz force-pushed the native-go-fuzz-integration branch 3 times, most recently from aa478dd to 605e181 Compare January 10, 2022 15:23
@AdamKorcz AdamKorcz force-pushed the native-go-fuzz-integration branch from 605e181 to 3783183 Compare January 10, 2022 15:27
@prestonvanloon
Copy link

is it possible to support fuzz targets with 2+ fuzz arguments?

Yes. This is already supported at this stage.

Thanks @AdamKorcz. To clarify, is it possible to support fuzz targets with 2+ fuzz args with the LLVMFuzzerTestOneInput interface?

Curious how that would work given that there is one input sent to n args.
Could you kindly add an example to the PR? Thanks in advance.

@AdamKorcz
Copy link
Collaborator Author

AdamKorcz commented Jan 10, 2022

is it possible to support fuzz targets with 2+ fuzz args with the LLVMFuzzerTestOneInput interface?

Yes. Native Go fuzzers with 1+ arguments are compatible with the current libfuzzer mode that OSS-fuzz will support for native Go fuzzers. Any native Go fuzzer with 1+ arguments of any of the natively supported types will be compiled with the LLVMFuzzerTestOneInput interface and built with -fsanitize=fuzzer. This is currently implemented and should be available when Go 1.18 is released. Hope it answers the question.

Curious how that would work given that there is one input sent to n args.

It is solved with a custom implementation of *testing.F.Fuzz() that checks which and how many arguments are passed, creates the arguments of those types from the byte array provided by libFuzzer and finally calls the original func(*testing.T, any) with those arguments. The implementation can be found here.

Could you kindly add an example to the PR? Thanks in advance.

Will do!

@oliverchang
Copy link
Collaborator

Any updates on this? It would be good to get this out and the documentation ready soon together with the Go 1.18 release date.

@AdamKorcz AdamKorcz force-pushed the native-go-fuzz-integration branch 2 times, most recently from 20da06b to 65e5046 Compare January 24, 2022 15:20
@AdamKorcz AdamKorcz force-pushed the native-go-fuzz-integration branch from 65e5046 to 48de2c6 Compare January 24, 2022 15:20
@AdamKorcz
Copy link
Collaborator Author

Any updates on this? It would be good to get this out and the documentation ready soon together with the Go 1.18 release date.

We are ready for review this week.

@AdamKorcz AdamKorcz marked this pull request as ready for review January 27, 2022 12:25
@AdamKorcz AdamKorcz requested a review from oliverchang January 27, 2022 12:26
Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!! @jonathanmetzman could you please take a pass as well?

docs/getting-started/new-project-guide/go_lang.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a first pass, most files LGTM except compile_native_go_fuzzer which I haven't finished reviewing.
To help me understand it, can you say why we rewrite the fuzz target?

docs/getting-started/new-project-guide/go_lang.md Outdated Show resolved Hide resolved
docs/getting-started/new-project-guide/go_lang.md Outdated Show resolved Hide resolved
$OUT/$target -test.coverprofile $DUMPS_DIR/$target.profdata &> $LOGS_DIR/$target.log

# The Go 1.18 fuzzers are renamed to "*_fuzz_.go" during "infra/helper.py build_fuzzers".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blech

infra/base-images/base-runner/coverage Outdated Show resolved Hide resolved
@@ -26,3 +26,16 @@ echo 'Set "PATH=$PATH:/root/.go/bin:$GOPATH/bin"'

go get -u github.com/mdempsky/go114-fuzz-build
ln -s $GOPATH/bin/go114-fuzz-build $GOPATH/bin/go-fuzz

go install golang.org/dl/gotip@latest \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused. Youre installing gotip in base-builder but using it in base-runner. Is this intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotip is also being used base-builder in the coverage build in compile_native_go_fuzzer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right but where is it coming from in base-runner then?

infra/base-images/base-builder/compile_native_go_fuzzer Outdated Show resolved Hide resolved
infra/base-images/base-builder/compile_native_go_fuzzer Outdated Show resolved Hide resolved
infra/base-images/base-builder/compile_native_go_fuzzer Outdated Show resolved Hide resolved
infra/base-images/base-builder/compile_native_go_fuzzer Outdated Show resolved Hide resolved
infra/base-images/base-builder/compile_native_go_fuzzer Outdated Show resolved Hide resolved
@jonathanmetzman
Copy link
Contributor

Adam could you check if these target binaries will contain the string LLVMFuzzerTestOneInput? If not I think it could break some things.

@AdamKorcz
Copy link
Collaborator Author

AdamKorcz commented Feb 2, 2022

To help me understand it, can you say why we rewrite the fuzz target?

2 things happen in rewrite_go_fuzz_harness:

  1. The file name is changed so it does not end with _test.go. This is so the fuzzer can be built with go build.
  2. *testing.F is replaced with *github.com/AdamKorcz/go-118-fuzz-build/utils.F to allow the fuzzer to generate any number of parameters of any type based on the byte array from libFuzzer. This is to accommodate the feature in Native Go Fuzzing where a fuzzer can have multiple arguments and they can be other types than the usual byte array. By using *github.com/AdamKorcz/go-118-fuzz-build/utils.F, any argument in a Native Go fuzzer will be created from the libFuzzer byte array.

@jonathanmetzman

@jonathanmetzman
Copy link
Contributor

random thought: I wonder if we should just be using golang's engine, would probably be worse and more effort on our part, but seems less hacky and perhaps they've optimized their fuzzer for Go code in a way libfuzzer isn't

@jonathanmetzman
Copy link
Contributor

To help me understand it, can you say why we rewrite the fuzz target?

2 things happen in rewrite_go_fuzz_harness:

  1. The file name is changed so it does not end with _test.go. This is so the fuzzer can be built with go build.
  2. *testing.F is replaced with *github.com/AdamKorcz/go-118-fuzz-build/utils.F to allow the fuzzer to generate any number of parameters of any type based on the byte array from libFuzzer. This is to accommodate the feature in Native Go Fuzzing where a fuzzer can have multiple arguments and they can be other types than the usual byte array. By using *github.com/AdamKorcz/go-118-fuzz-build/utils.F, any argument in a Native Go fuzzer will be created from the libFuzzer byte array.

@jonathanmetzman

Ah so we're not really using golang's native fuzzer. Two questions:

  1. are the command line arguments different from the native fuzzer?
  2. Is the corpus compatible between the two?

@AdamKorcz
Copy link
Collaborator Author

random thought: I wonder if we should just be using golang's engine, would probably be worse and more effort on our part, but seems less hacky and perhaps they've optimized their fuzzer for Go code in a way libfuzzer isn't

We discussed this with @oliverchang and the Go team. The conclusion was to go with this solution for now and perhaps switch to the Native Go fuzzing engine at a later stage. Several pros and cons came up, but on the pros side of the libFuzzer approach is that support for Native Go harnesses is available in OSS-fuzz when Go 1.18 releases, and users can start running their fuzzers continuously right away. In addition to that: No infra/runtime changes are required on the OSS-fuzz side. On the con side: Avoiding the rewrites in this PR, having similar UX between local fuzzing and continuous fuzzing.

Another pro of the approach in this PR is that it follows the paradigm of how go-fuzz is currently used in OSS-fuzz: i.e. the existing go-fuzz already has their own engine but the oss-fuzz fuzzers run by way of libFuzzer.

By the way, related to this we thought it might be interesting to implement go fuzz engines in fuzzbench as it seems there are a bunch now.

@AdamKorcz
Copy link
Collaborator Author

  1. are the command line arguments different from the native fuzzer?
  2. Is the corpus compatible between the two?

Am not entirely sure if you're asking about the native go-fuzzing engine in general or the additions in this PR? As in, the corpus is not compatible by default but we have added support for this (see the feature list in the initial description).

Note that we have tested this work in oss-fuzz by way of a test project here: https://github.com/google/oss-fuzz/tree/master/projects/testing-native-go-fuzzing.

@jonathanmetzman
Copy link
Contributor

jonathanmetzman commented Feb 2, 2022

random thought: I wonder if we should just be using golang's engine, would probably be worse and more effort on our part, but seems less hacky and perhaps they've optimized their fuzzer for Go code in a way libfuzzer isn't

We discussed this with @oliverchang and the Go team. The conclusion was to go with this solution for now and perhaps switch to the Native Go fuzzing engine at a later stage. Several pros and cons came up, but on the pros side of the libFuzzer approach is that support for Native Go harnesses is available in OSS-fuzz when Go 1.18 releases, and users can start running their fuzzers continuously right away. In addition to that: No infra/runtime changes are required on the OSS-fuzz side. On the con side: Avoiding the rewrites in this PR, having similar UX between local fuzzing and continuous fuzzing.

Makes sense, I hadn't seen what was decided.

Another pro of the approach in this PR is that it follows the paradigm of how go-fuzz is currently used in OSS-fuzz: i.e. the existing go-fuzz already has their own engine but the oss-fuzz fuzzers run by way of libFuzzer.

ACK.

By the way, related to this we thought it might be interesting to implement go fuzz engines in fuzzbench as it seems there are a bunch now.

+1

@DavidKorczynski
Copy link
Collaborator

Note that we have tested this work in oss-fuzz by way of a test project here: https://github.com/google/oss-fuzz/tree/master/projects/testing-native-go-fuzzing.

In addition we have tested on the Go-ethereum and Istio projects as well (as described in the PR text), i.e. we have added native go-fuzzers to those projects and their have been running for a month or so

@jonathanmetzman
Copy link
Contributor

  1. are the command line arguments different from the native fuzzer?
  2. Is the corpus compatible between the two?

Am not entirely sure if you're asking about the native go-fuzzing engine in general or the additions in this PR? As in, the corpus is not compatible by default but we have added support for this (see the feature list in the initial description).

Note that we have tested this work in oss-fuzz by way of a test project here: https://github.com/google/oss-fuzz/tree/master/projects/testing-native-go-fuzzing.

Perhaps I'm being unclear but here is what i mean.
Suppose a go developer runs the fuzzer locally gets some interesting testcases (ones that caused crashes) and then includes this in OSS-Fuzz as a seed corpus.
Will these testcases be interpreted the same by libFuzzer. What I'm really getting at is: is the way libfuzzer (with your shim) and go native fuzzing represent multi-arg testcases the same?

Copy link
Contributor

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
The change seems fine to me based on what was decided for this feature.

@AdamKorcz
Copy link
Collaborator Author

What I'm really getting at is: is the way libfuzzer (with your shim) and go native fuzzing represent multi-arg testcases the same?

We have not performed this test, but my guess is that the answer is no. We're happy to explore this, and, perhaps do it simultaneously with migrating the native engine itself

@jonathanmetzman
Copy link
Contributor

jonathanmetzman commented Feb 2, 2022

What I'm really getting at is: is the way libfuzzer (with your shim) and go native fuzzing represent multi-arg testcases the same?

We have not performed this test, but my guess is that the answer is no. We're happy to explore this, and, perhaps do it simultaneously with migrating the native engine itself

I think this is somewhat of a problem. most likely users won't noticed this (or the fact that .options file may also not work as they expect). Is this the same behavior wrt to libfuzzer for gofuzz targets?

@AdamKorcz
Copy link
Collaborator Author

I think this is somewhat of a problem. most likely users won't noticed this (or the fact that .options file may also not work as they expect).

I agree. The current implementation is focused on a first step in this context, i.e. OSS-fuzz will support running Native Go fuzzers instantly. There are more steps in maturing it and this is one of those for sure!

Note that at the moment Go 1.18 fuzzing has very few users: I have found a single OSS-fuzz project with two 1.18 fuzzers upstream.

Is this the same behavior wrt to libfuzzer for gofuzz targets?

No. These can be used interchangeably.

@AdamKorcz
Copy link
Collaborator Author

Adam could you check if these target binaries will contain the string LLVMFuzzerTestOneInput?

Can confirm all target binaries this was tested on (all mentioned in PR text) contain the string "LLVMFuzzerTestOneInput".

@oliverchang oliverchang merged commit 4fdde05 into google:master Feb 8, 2022
@AdamKorcz AdamKorcz changed the title [draft] Integrate native go fuzzing Integrate native go fuzzing Feb 8, 2022
MartinPetkov pushed a commit to MartinPetkov/oss-fuzz that referenced this pull request Aug 15, 2022
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

Successfully merging this pull request may close these issues.

5 participants