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

Better support for fuzzing Go, remaining - code coverage #2714

Closed
5 tasks done
Dor1s opened this issue Aug 16, 2019 · 9 comments
Closed
5 tasks done

Better support for fuzzing Go, remaining - code coverage #2714

Dor1s opened this issue Aug 16, 2019 · 9 comments
Assignees
Labels

Comments

@Dor1s
Copy link
Contributor

Dor1s commented Aug 16, 2019

As of today, ClusterFuzz supports fuzzing Go in the gofuzz_libfuzzer mode: https://github.com/dvyukov/go-fuzz#libfuzzer-support

But it's not really convenient to integrate a new Golang project. I think we should do the following:

  1. Upgrade the base-builder image to include a recent version of Go toolchain and go-fuzz, so that it was possible to run go and go-fuzz-build commands from build.sh without any problems.

  2. Document it :)

Action items

Dor1s added a commit that referenced this issue Aug 21, 2019
* [infra] Update base-builder image to support go-fuzz (#2714).

* address review comments
Dor1s added a commit that referenced this issue Sep 17, 2019
)

* [docs] Add "Integrating a Go project" documentation page (#2714).

* rephrase go-fuzz mode description
@Dor1s Dor1s mentioned this issue Jan 17, 2020
@catenacyber
Copy link
Contributor

I am not sure if it fits here, but I would like to see "LAST TESTED REVISION" on oss-fuzz.com for golang projects
The trick is, I think, that build.sh uses go get instead of git clone
Example : https://oss-fuzz.com/testcase-detail/5453012747419648

@Dor1s
Copy link
Contributor Author

Dor1s commented Jan 27, 2020

Interesting! I guess you meant Dockerfile using go get, e.g.

RUN go get github.com/google/gonids

Yeah, looks like our srcmap script should grab revisions from the repos isntalled via go get as well. I'll upload a patch, thanks for noticing this!

@catenacyber
Copy link
Contributor

Exactly

Thanks @Dor1s for patching, I did not know where to look, or I would have proposed a patch myself ;-)

@inferno-chromium
Copy link
Collaborator

Tracking items:

  1. Switch to native go instrumentation.
  2. Make projects use the native go binary for building.
  3. Fix documentation - https://google.github.io/oss-fuzz/getting-started/new-project-guide/go-lang
  4. Add how to write fuzz targets (examples). this section has none - https://google.github.io/oss-fuzz/getting-started/new-project-guide/go-lang/#go-fuzz-support

fyi - @lukasz-milewski @mdempsky

@Dor1s
Copy link
Contributor Author

Dor1s commented Apr 12, 2020

Thanks @inferno-chromium, I've updated the issue description with more of these.

Also added code coverage (#2817) and srcmap support (#3355).

Regarding

  1. Add how to write fuzz targets (examples). this section has none - https://google.github.io/oss-fuzz/getting-started/new-project-guide/go-lang/#go-fuzz-support

I'm not sure this would belong to OSS-Fuzz repo, as OSS-Fuzz docs focus on using OSS-Fuzz and do not cover how to write fuzz targets in general (e.g. https://google.github.io/oss-fuzz/getting-started/new-project-guide/ also doesn't have any C/C++ examples).

https://github.com/google/fuzzing might be a better place, or we can point to https://godoc.org/github.com/google/gofuzz

@Dor1s
Copy link
Contributor Author

Dor1s commented Apr 14, 2020

@inferno-chromium thanks a ton for adding the new instrumentation support (#3633) and migrating all Go projects to use it (#3638). Marking the first AI as done!

If you plan to work more on this, I think we should also unify the compile_fuzzer function which is copy-pasted in all build.sh files now and make it a common script inside base-builder (+ documentation) -- this is the second action item. Otherwise, all new projects will be also copy-pasting it, and the next migration will be more annoying.

Updating the docs after that is the third item :)

@inferno-chromium
Copy link
Collaborator

I thought about 2) and 3). i think it will create more confusion for folks when they want to do ideal integration (which we recommend, most projects should have build.sh in their repo). They would have to look this helper script and then integrate it as well. for 2-3 build commands, it feels like a hassle. e.g. https://go.googlesource.com/protobuf/+/d8bc21f7e13fa476be55b17983bd5d43ad8c7121/internal/fuzz/oss-fuzz-build.sh. i prefer to keep it in current way.

@Dor1s
Copy link
Contributor Author

Dor1s commented Apr 16, 2020

I thought about 2) and 3). i think it will create more confusion for folks when they want to do ideal integration

Ah, right, great point!

Dor1s added a commit that referenced this issue Apr 16, 2020
…2714). (#3657)

* [Go] remove "-lpthread" flag from "compile_fuzzer" and fix comments (#2714).

* fix unrelated go-dns and gonids errors

* simplify golang/build.sh as per Abhishek's comment
Dor1s added a commit that referenced this issue Apr 17, 2020
* [infra] Improve srcmap support for Go projects (#3355, #2714).

* address review feedback
@inferno-chromium inferno-chromium changed the title Better support for fuzzing Go Better support for fuzzing Go, remaining - code coverage May 31, 2020
@inferno-chromium inferno-chromium self-assigned this Aug 19, 2020
Dor1s added a commit that referenced this issue Nov 20, 2020
…4671)

* Golang coverage summary for each fuzz target

* Document usage of compile_go_fuzzer

* update the documentation change

Co-authored-by: Max Moroz <mmoroz@chromium.org>
@jonathanmetzman
Copy link
Contributor

I think @catenacyber completed this by adding code coverage support for go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants