Add a fuzzer for libgit2 by nelhage · Pull Request #1604 · google/oss-fuzz

nelhage · 2018-07-10T03:19:41Z

Add an initial libgit2 fuzzer for the git fetch client. This has found a number of bugs already in local running:

I am not a libgit2 developer, but upstream has expressed interest in landing this fuzzer: libgit2/libgit2#4697 ; I figured I would start by trying to land an integration here, and we can move pieces into libgit2.git if and when it's stable here.

I've tagged @pks-t, a libgit2 developer who's expressed their interest, as the primary contact for now.

Slightly tweaked network dump of a `git clone` on a tiny repo.

googlebot · 2018-07-10T03:19:44Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

nelhage · 2018-07-10T03:21:52Z

I've signed the CLA.

googlebot · 2018-07-10T03:21:55Z

CLAs look good, thanks!

inferno-chromium

@pks-t - can you do a review pass on the download_refs fuzz target and see that the inits and calls are done correctly. And are you ok with this being integrated in OSS-Fuzz.

inferno-chromium · 2018-07-10T03:27:26Z

+RUN git clone --depth 1 -b oss-fuzz https://github.com/libgit2/libgit2 libgit2
+WORKDIR libgit2
+
+COPY fuzz_download_refs.cc build.sh $SRC/


fuzz_download_refs.cc, build.sh and corpora should be later moved to upstram repo. See https://github.com/google/oss-fuzz/blob/master/docs/ideal_integration.md

inferno-chromium · 2018-07-10T03:29:51Z

@@ -0,0 +1,3 @@
+homepage: "https://libgit2.github.com/"
+auto_ccs: "nelhage@nelhage.com"
+primary_contact: "Patrick Steinhardt <ps@pks.im>"


This needs to be just email.

inferno-chromium · 2018-07-10T03:30:25Z

+make install
+
+$CXX $CXXFLAGS -std=c++11 -I"$WORK/include" \
+    /src/fuzz_download_refs.cc -o $OUT/download_refs \


Instead of fuzz_download_refs, better to call it download_refs_fuzzer for both filename and fuzzer name.

inferno-chromium · 2018-07-10T03:33:34Z

+  static git_repository *repo = nullptr;
+  if (repo == nullptr) {
+      git_libgit2_init();
+      char tmp[] = "/tmp/git2.XXXXXX";


Space in /tmp is limited to 64 mb, hope this is not a blocker.

In my tests I've yet to see this get beyond a few 10s of kb, so that should be fine.

I had left in a `-b oss-fuzz` from testing off my fork

alex · 2018-07-10T03:54:10Z

-primary_contact: "Patrick Steinhardt <ps@pks.im>"
+auto_ccs:
+  - "nelhage@nelhage.com"
+primary_contact: "<ps@pks.im>"


pks-t · 2018-07-10T07:02:26Z

I'm currently out of town and thus cannot do any reviews. But I'll do so on coming Thursday.

kcc · 2018-07-10T22:25:18Z

This target quickly dies with an OOM, but otherwise looks good.

nelhage · 2018-07-10T22:27:26Z

Ah, that OOM might be fixed by nelhage/libgit2@a453f6f which I added locally to test against but forgot to submit upstream. I'll send a PR to libgit2 later today.

kcc · 2018-07-10T22:31:46Z

often such OOMs are easily exploitable for DDoS and are thus worth fixing for real, not just for fuzzing.
IDK if this is important here.

nelhage · 2018-07-10T22:54:10Z

Yeah, per comments there was already a deliberately-selected 4GB limit, which seems at least plausibly an OK default, but doesn't work for fuzzing. I'll open a PR and see if upstream thinks it needs to be configurable or what.

kcc · 2018-07-10T22:57:15Z

I see mallocs of huge size, much more than 4Gb:

==201786== ERROR: libFuzzer: out-of-memory (malloc(34091302904))
   To change the out-of-memory limit use -rss_limit_mb=<N>

    #0 0x4fad33 in __sanitizer_print_stack_trace /src/llvm/projects/compiler-rt/lib/asan/asan_stack.cc:38
    #1 0x5b2a9a in fuzzer::PrintStackTrace() /src/libfuzzer/FuzzerUtil.cpp:206:5
    #2 0x5522f9 in fuzzer::Fuzzer::HandleMalloc(unsigned long) /src/libfuzzer/FuzzerLoop.cpp:132:3
    #3 0x5520d0 in fuzzer::MallocHook(void const volatile*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:101:6
    #4 0x504741 in __sanitizer::RunMallocHooks(void const*, unsigned long) /src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common.cc:300
    #5 0x427097 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) /src/llvm/projects/compiler-rt/lib/asan/asan_allocator.cc:544
    #6 0x4281e7 in __asan::asan_realloc(void*, unsigned long, __sanitizer::BufferedStackTrace*) /src/llvm/projects/compiler-rt/lib/asan/asan_allocator.cc:885
    #7 0x4ebba9 in realloc /src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:141
    #8 0x836882 in stdalloc__reallocarray /src/libgit2/src/stdalloc.c:95:10
    #9 0x610893 in resize_vector /src/libgit2/src/vector.c:35:17
    #10 0x6116f7 in git_vector_init /src/libgit2/src/vector.c:108:9
    #11 0x8d5e09 in git_indexer_append /src/libgit2/src/indexer.c:571:7
    #12 0x7fcb25 in pack_backend__writepack_append /src/libgit2/src/odb_pack.c:494:9
    #13 0x934cd9 in git_smart__download_pack /src/libgit2/src/transports/smart_protocol.c:619:14
    #14 0x8c1bac in git_fetch_download_pack /src/libgit2/src/fetch.c:148:9
    #15 0x755d23 in git_remote_download /src/libgit2/src/remote.c:932:9
    #16 0x52e967 in LLVMFuzzerTestOneInput /src/download_refs_fuzzer.cc:150:3

pks-t · 2018-07-13T05:53:44Z

The code looks fine to me, thanks for working on this, @nelhage. That being said, I'd strongly favor an implementation in C90 and avoid all usage of C++. The reason is that I'd like to pull this code into libgit2 itself at some point, such that it becomes easier for us to maintain fuzzers and/or execute them locally without having to fetch any external projects first. I don't know if it is customary to have the actual fuzzer implementation in the oss-fuzz or upstream project, though.

I'd be fine with having the fuzzers here first and doing the work of pulling them up into libgit2 myself, if you wish, including the transformation to C90. Please let me know how you'd like to handle it.

/cc @ethomson

nelhage · 2018-07-13T14:22:33Z

@pks-t Best practice (per https://github.com/google/oss-fuzz/blob/master/docs/ideal_integration.md) is for fuzzers to live in the upstream repo in the long run. I was planning to merge them here since that felt like an easier v1, and then attempt to import them into libgit2.git once everything is working. I'd be happy to port to C90 when I do so; The C++isms here are mostly overkill caused by me mostly writing C++ these days :)

kcc · 2018-07-13T15:13:00Z

Let's see how it works. Most likely, it will get stuck in the OOM above and that will have to be fixed in order to proceed.

kcc · 2018-07-13T19:35:14Z

And here it is: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9402
With this OOM happening this often, fuzzing will be very slow. Please try to get it fixed.

nelhage · 2018-07-13T20:46:56Z

I've got a PR out that will fix it, pending some design discussion: libgit2/libgit2#4721

If we can't sort a proper API soon I might re-propose a fuzzer ifdef as an interim fix.

* Add a libgit2 fuzzer for the `git fetch` client * Use a fresh remote each time * Build fewer things, use fewer deps * no ssh, either * Add a corpus with one file. Slightly tweaked network dump of a `git clone` on a tiny repo. * auto_ccs is a list * Rename the fuzzer * only email in project.yaml * Use `master` I had left in a `-b oss-fuzz` from testing off my fork

nelhage added 5 commits July 10, 2018 03:12

Add a libgit2 fuzzer for the git fetch client

75e0cf1

Use a fresh remote each time

5ecf933

Build fewer things, use fewer deps

9daf43d

no ssh, either

329ac04

Add a corpus with one file.

57db0f6

Slightly tweaked network dump of a `git clone` on a tiny repo.

auto_ccs is a list

cd586ec

inferno-chromium reviewed Jul 10, 2018

View reviewed changes

nelhage added 3 commits July 10, 2018 03:50

Rename the fuzzer

7bccddb

only email in project.yaml

04c33b9

Use master

caa9304

I had left in a `-b oss-fuzz` from testing off my fork

alex reviewed Jul 10, 2018

View reviewed changes

nelhage mentioned this pull request Jul 11, 2018

Add a configurable limit to the max pack size that will be indexed libgit2/libgit2#4721

Merged

kcc merged commit c0661ee into google:master Jul 13, 2018

Conversation

nelhage commented Jul 10, 2018

Uh oh!

googlebot commented Jul 10, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

nelhage commented Jul 10, 2018

Uh oh!

googlebot commented Jul 10, 2018

Uh oh!

inferno-chromium left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inferno-chromium Jul 10, 2018

Choose a reason for hiding this comment

Uh oh!

inferno-chromium Jul 10, 2018

Choose a reason for hiding this comment

Uh oh!

inferno-chromium Jul 10, 2018

Choose a reason for hiding this comment

Uh oh!

inferno-chromium Jul 10, 2018

Choose a reason for hiding this comment

Uh oh!

nelhage Jul 10, 2018

Choose a reason for hiding this comment

Uh oh!

alex Jul 10, 2018

Choose a reason for hiding this comment

Uh oh!

pks-t commented Jul 10, 2018 via email

Uh oh!

kcc commented Jul 10, 2018

Uh oh!

nelhage commented Jul 10, 2018

Uh oh!

kcc commented Jul 10, 2018

Uh oh!

nelhage commented Jul 10, 2018

Uh oh!

kcc commented Jul 10, 2018

Uh oh!

pks-t commented Jul 13, 2018

Uh oh!

nelhage commented Jul 13, 2018

Uh oh!

kcc commented Jul 13, 2018

Uh oh!

kcc commented Jul 13, 2018

Uh oh!

nelhage commented Jul 13, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

inferno-chromium left a comment •

edited

Loading