threads / rfc / 49486

RFC patch, 2 partsadd fuzzing targets for use with LLVM libFuzzer

Subject: [RFC PATCH 0/2] add fuzzing targets for use with LLVM libFuzzer

## tl;dr

10 messages between Oct 4, 2018 and Oct 16, 2018. Diffs are folded; open one to read it.

replies: 9people: 2as markdown or json

Josh Steadmon· Oct 4, 2018, 23:01 UTC · lore

libFuzzer[1] is a fuzzing engine included in recent versions of LLVM. It is used by OSS-Fuzz[2] for continuous fuzzing of OSS projects.

This series adds two basic fuzzing targets covering packfile header and index code. It is not particularly portable, and requires the use of LLVM v4.0 (the latest version available on my workstation). I would particularly appreciate advice on how to make the Makefile more portable.

[1]: https://llvm.org/docs/LibFuzzer.html [2]: https://github.com/google/oss-fuzz

Josh Steadmon (2):
  fuzz: Add basic fuzz testing target.
  fuzz: Add fuzz testing for packfile indices.
 .gitignore          |  3 +++
 Makefile            | 33 ++++++++++++++++++++++++++++++++-
 fuzz-pack-headers.c | 14 ++++++++++++++
 fuzz-pack-idx.c     | 13 +++++++++++++
 packfile.c          | 44 +++++++++++++++++++++++++-------------------
 packfile.h          | 13 +++++++++++++
 6 files changed, 100 insertions(+), 20 deletions(-)
 create mode 100644 fuzz-pack-headers.c
 create mode 100644 fuzz-pack-idx.c
-- 
2.19.0.605.g01d371f741-goog
Josh Steadmon· Oct 4, 2018, 23:01 UTC · re: Josh Steadmon · lore

[RFC PATCH 1/2] fuzz: Add basic fuzz testing target.

Signed-off-by: Josh Steadmon <steadmon@google.com>
---
 .gitignore          |  2 ++
 Makefile            | 30 +++++++++++++++++++++++++++++-
 fuzz-pack-headers.c | 14 ++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 fuzz-pack-headers.c
Show changes to 3 files +45 −1

.gitignore, Makefile, fuzz-pack-headers.c

diff --git a/.gitignore b/.gitignore
index 9d1363a1eb..87a28b3115 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,5 @@
+/fuzz_corpora
+/fuzz-pack-headers
 /GIT-BUILD-OPTIONS
 /GIT-CFLAGS
 /GIT-LDFLAGS
diff --git a/Makefile b/Makefile
index 13e1c52478..10bb82b115 100644
--- a/Makefile
+++ b/Makefile
@@ -590,6 +590,8 @@ XDIFF_OBJS =
 VCSSVN_OBJS =
 GENERATED_H =
 EXTRA_CPPFLAGS =
+FUZZ_OBJS =
+FUZZ_PROGRAMS =
 LIB_OBJS =
 PROGRAM_OBJS =
 PROGRAMS =
@@ -682,6 +684,10 @@ SCRIPTS = $(SCRIPT_SH_INS) \
 
 ETAGS_TARGET = TAGS
 
+FUZZ_OBJS += fuzz-pack-headers.o
+
+FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))
+
 # Empty...
 EXTRA_PROGRAMS =
 
@@ -2250,6 +2256,7 @@ TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST
 OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
 	$(XDIFF_OBJS) \
 	$(VCSSVN_OBJS) \
+	$(FUZZ_OBJS) \
 	common-main.o \
 	git.o
 ifndef NO_CURL
@@ -2931,7 +2938,7 @@ profile-clean:
 cocciclean:
 	$(RM) contrib/coccinelle/*.cocci.patch*
 
-clean: profile-clean coverage-clean cocciclean
+clean: profile-clean coverage-clean cocciclean fuzz-clean
 	$(RM) *.res
 	$(RM) $(OBJECTS)
 	$(RM) $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
@@ -3061,3 +3068,24 @@ cover_db: coverage-report
 cover_db_html: cover_db
 	cover -report html -outputdir cover_db_html cover_db
 
+
+### Fuzz testing
+#
+.PHONY: fuzz-clean fuzz-objs fuzz-compile
+
+FUZZ_CFLAGS = $(CFLAGS) -fsanitize-coverage=trace-pc-guard -fsanitize=address
+FUZZ_LDFLAGS = $(FUZZ_CFLAGS)
+
+
+fuzz-clean:
+	$(RM) $(FUZZ_PROGRAMS) $(FUZZ_OBJS)
+
+fuzz-objs: $(FUZZ_OBJS)
+
+fuzz-compile:
+	$(MAKE) CC=clang LD=clang CFLAGS="$(FUZZ_CFLAGS)" \
+		LDFLAGS="$(FUZZ_LDFLAGS)" all fuzz-objs
+
+$(FUZZ_PROGRAMS): fuzz-compile
+	clang++ $(FUZZ_LDFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) $(XDIFF_OBJS) \
+		$(EXTLIBS) git.o $@.o /usr/lib/llvm-4.0/lib/libFuzzer.a -o $@
diff --git a/fuzz-pack-headers.c b/fuzz-pack-headers.c
new file mode 100644
index 0000000000..99da1d0fd3
--- /dev/null
+++ b/fuzz-pack-headers.c
@@ -0,0 +1,14 @@
+#include "packfile.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+	enum object_type type;
+	unsigned long len;
+
+	unpack_object_header_buffer((const unsigned char *)data,
+				    (unsigned long)size, &type, &len);
+
+	return 0;
+}
-- 
2.19.0.605.g01d371f741-goog
Junio C Hamano· Oct 10, 2018, 02:14 UTC · re: Josh Steadmon · lore

Re: [RFC PATCH 1/2] fuzz: Add basic fuzz testing target.

Josh Steadmon <steadmon@google.com> writes:
Show 8 quoted lines
> +FUZZ_OBJS += fuzz-pack-headers.o
> +
> +FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))
> +
> ...
> +### Fuzz testing
> +#
> +.PHONY: fuzz-clean fuzz-objs fuzz-compile

I take it that you anticipate the fuzz programs in the future all be named fuzz-$(blah), whose source is fuzz-$(blah).o (even though we may grow some common code that may be linked with them, which can be done by tweaking the rule for the $(FUZZ_PROGRAMS) target). Am I reading you correctly? Would fuzz-{clean,objs,compile} risk squatting on nicer names we may want to use for $(blah) down the line?

> + ...
> +$(FUZZ_PROGRAMS): fuzz-compile
> +	clang++ $(FUZZ_LDFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) $(XDIFF_OBJS) \
> +		$(EXTLIBS) git.o $@.o /usr/lib/llvm-4.0/lib/libFuzzer.a -o $@

Is the expected usage pattern to know a single fuzz-* program the builder wants to build, to run "make fuzz-pack-headers"? If not, it also would be a good idea to have something like

    fuzz-build-all:: $(FUZZ_PROGRAMS)
    .PHONY: fuzz-build-all
perhaps?

Also, in the final version we unleash to general developer audience, we'd want to support "make V=1" (and "make" that is "$(QUIET)").

Josh Steadmon· Oct 13, 2018, 00:59 UTC · re: Junio C Hamano · lore

Re: [RFC PATCH 1/2] fuzz: Add basic fuzz testing target.

On 2018.10.10 11:14, Junio C Hamano wrote:
Show 17 quoted lines
> Josh Steadmon <steadmon@google.com> writes:
> 
> > +FUZZ_OBJS += fuzz-pack-headers.o
> > +
> > +FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))
> > +
> > ...
> > +### Fuzz testing
> > +#
> > +.PHONY: fuzz-clean fuzz-objs fuzz-compile
> 
> I take it that you anticipate the fuzz programs in the future all
> be named fuzz-$(blah), whose source is fuzz-$(blah).o (even though
> we may grow some common code that may be linked with them, which can
> be done by tweaking the rule for the $(FUZZ_PROGRAMS) target).  Am I
> reading you correctly?  Would fuzz-{clean,objs,compile} risk squatting
> on nicer names we may want to use for $(blah) down the line?

Yes, that's correct. I've reworked the rules to be more compatible with how OSS-Fuzz expects to build these targets, and now "fuzz-all" is the only remaining special target.

Show 16 quoted lines
> > + ...
> > +$(FUZZ_PROGRAMS): fuzz-compile
> > +	clang++ $(FUZZ_LDFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) $(XDIFF_OBJS) \
> > +		$(EXTLIBS) git.o $@.o /usr/lib/llvm-4.0/lib/libFuzzer.a -o $@
> 
> Is the expected usage pattern to know a single fuzz-* program the
> builder wants to build, to run "make fuzz-pack-headers"?  If not, it
> also would be a good idea to have something like
> 
>     fuzz-build-all:: $(FUZZ_PROGRAMS)
>     .PHONY: fuzz-build-all
> 
> perhaps?
> 
> Also, in the final version we unleash to general developer audience,
> we'd want to support "make V=1" (and "make" that is "$(QUIET)").
Done and done.
Josh Steadmon· Oct 4, 2018, 23:01 UTC · re: Josh Steadmon · lore

[RFC PATCH 2/2] fuzz: Add fuzz testing for packfile indices.

Signed-off-by: Josh Steadmon <steadmon@google.com>
---
 .gitignore      |  1 +
 Makefile        |  5 ++++-
 fuzz-pack-idx.c | 13 +++++++++++++
 packfile.c      | 44 +++++++++++++++++++++++++-------------------
 packfile.h      | 13 +++++++++++++
 5 files changed, 56 insertions(+), 20 deletions(-)
 create mode 100644 fuzz-pack-idx.c
Show changes to 5 files +56 −20

.gitignore, Makefile, fuzz-pack-idx.c, packfile.c, packfile.h

diff --git a/.gitignore b/.gitignore
index 87a28b3115..64b3377d40 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,6 @@
 /fuzz_corpora
 /fuzz-pack-headers
+/fuzz-pack-idx
 /GIT-BUILD-OPTIONS
 /GIT-CFLAGS
 /GIT-LDFLAGS
diff --git a/Makefile b/Makefile
index 10bb82b115..f7e6be8f19 100644
--- a/Makefile
+++ b/Makefile
@@ -685,6 +685,7 @@ SCRIPTS = $(SCRIPT_SH_INS) \
 ETAGS_TARGET = TAGS
 
 FUZZ_OBJS += fuzz-pack-headers.o
+FUZZ_OBJS += fuzz-pack-idx.o
 
 FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))
 
@@ -3071,7 +3072,7 @@ cover_db_html: cover_db
 
 ### Fuzz testing
 #
-.PHONY: fuzz-clean fuzz-objs fuzz-compile
+.PHONY: fuzz-clean fuzz-objs fuzz-compile fuzz-all
 
 FUZZ_CFLAGS = $(CFLAGS) -fsanitize-coverage=trace-pc-guard -fsanitize=address
 FUZZ_LDFLAGS = $(FUZZ_CFLAGS)
@@ -3089,3 +3090,5 @@ fuzz-compile:
 $(FUZZ_PROGRAMS): fuzz-compile
 	clang++ $(FUZZ_LDFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) $(XDIFF_OBJS) \
 		$(EXTLIBS) git.o $@.o /usr/lib/llvm-4.0/lib/libFuzzer.a -o $@
+
+fuzz-all: $(FUZZ_PROGRAMS)
diff --git a/fuzz-pack-idx.c b/fuzz-pack-idx.c
new file mode 100644
index 0000000000..0c3d777aac
--- /dev/null
+++ b/fuzz-pack-idx.c
@@ -0,0 +1,13 @@
+#include "object-store.h"
+#include "packfile.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+	struct packed_git p;
+
+	load_idx("fuzz-input", GIT_SHA1_RAWSZ, (void *)data, size, &p);
+
+	return 0;
+}
diff --git a/packfile.c b/packfile.c
index 841b36182f..86074a76e9 100644
--- a/packfile.c
+++ b/packfile.c
@@ -80,10 +80,8 @@ void pack_report(void)
 static int check_packed_git_idx(const char *path, struct packed_git *p)
 {
 	void *idx_map;
-	struct pack_idx_header *hdr;
 	size_t idx_size;
-	uint32_t version, nr, i, *index;
-	int fd = git_open(path);
+	int fd = git_open(path), ret;
 	struct stat st;
 	const unsigned int hashsz = the_hash_algo->rawsz;
 
@@ -101,16 +99,32 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 	idx_map = xmmap(NULL, idx_size, PROT_READ, MAP_PRIVATE, fd, 0);
 	close(fd);
 
-	hdr = idx_map;
+	ret = load_idx(path, hashsz, idx_map, idx_size, p);
+
+	if (ret)
+		munmap(idx_map, idx_size);
+
+	return ret;
+}
+
+int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
+	     size_t idx_size, struct packed_git *p)
+{
+	struct pack_idx_header *hdr = idx_map;
+	uint32_t version, nr, i, *index;
+
+	if (idx_size < 4 * 256 + hashsz + hashsz)
+		return error("index file %s is too small", path);
+	if (idx_map == NULL)
+		return error("empty data");
+
 	if (hdr->idx_signature == htonl(PACK_IDX_SIGNATURE)) {
 		version = ntohl(hdr->idx_version);
-		if (version < 2 || version > 2) {
-			munmap(idx_map, idx_size);
+		if (version < 2 || version > 2)
 			return error("index file %s is version %"PRIu32
 				     " and is not supported by this binary"
 				     " (try upgrading GIT to a newer version)",
 				     path, version);
-		}
 	} else
 		version = 1;
 
@@ -120,10 +134,8 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 		index += 2;  /* skip index header */
 	for (i = 0; i < 256; i++) {
 		uint32_t n = ntohl(index[i]);
-		if (n < nr) {
-			munmap(idx_map, idx_size);
+		if (n < nr)
 			return error("non-monotonic index %s", path);
-		}
 		nr = n;
 	}
 
@@ -135,10 +147,8 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 		 *  - hash of the packfile
 		 *  - file checksum
 		 */
-		if (idx_size != 4*256 + nr * (hashsz + 4) + hashsz + hashsz) {
-			munmap(idx_map, idx_size);
+		if (idx_size != 4 * 256 + nr * (hashsz + 4) + hashsz + hashsz)
 			return error("wrong index v1 file size in %s", path);
-		}
 	} else if (version == 2) {
 		/*
 		 * Minimum size:
@@ -157,20 +167,16 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 		unsigned long max_size = min_size;
 		if (nr)
 			max_size += (nr - 1)*8;
-		if (idx_size < min_size || idx_size > max_size) {
-			munmap(idx_map, idx_size);
+		if (idx_size < min_size || idx_size > max_size)
 			return error("wrong index v2 file size in %s", path);
-		}
 		if (idx_size != min_size &&
 		    /*
 		     * make sure we can deal with large pack offsets.
 		     * 31-bit signed offset won't be enough, neither
 		     * 32-bit unsigned one will be.
 		     */
-		    (sizeof(off_t) <= 4)) {
-			munmap(idx_map, idx_size);
+		    (sizeof(off_t) <= 4))
 			return error("pack too large for current definition of off_t in %s", path);
-		}
 	}
 
 	p->index_version = version;
diff --git a/packfile.h b/packfile.h
index 442625723d..6c4037605d 100644
--- a/packfile.h
+++ b/packfile.h
@@ -164,4 +164,17 @@ extern int has_pack_index(const unsigned char *sha1);
  */
 extern int is_promisor_object(const struct object_id *oid);
 
+/*
+ * Expose a function for fuzz testing.
+ *
+ * load_idx() parses a block of memory as a packfile index and puts the results
+ * into a struct packed_git.
+ *
+ * This function should not be used directly. It is exposed here only so that we
+ * have a convenient entry-point for fuzz testing. For real uses, you should
+ * probably use open_pack_index() or parse_pack_index() instead.
+ */
+extern int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
+		    size_t idx_size, struct packed_git *p);
+
 #endif
-- 
2.19.0.605.g01d371f741-goog
Junio C Hamano· Oct 10, 2018, 02:19 UTC · re: Josh Steadmon · lore

Re: [RFC PATCH 2/2] fuzz: Add fuzz testing for packfile indices.

Josh Steadmon <steadmon@google.com> writes:
Show 9 quoted lines
>  ### Fuzz testing
>  #
> -.PHONY: fuzz-clean fuzz-objs fuzz-compile
> +.PHONY: fuzz-clean fuzz-objs fuzz-compile fuzz-all
> ...
>  FUZZ_CFLAGS = $(CFLAGS) -fsanitize-coverage=trace-pc-guard -fsanitize=address
> ...
> +
> +fuzz-all: $(FUZZ_PROGRAMS)

I guess I read your mind ;-) Please do this in 1/2 instead of adding it at 2/2 as "oops, we'd need it more and more as we add these".

steadmon@google.com· Oct 13, 2018, 00:58 UTC · re: Josh Steadmon · lore

[PATCH v2 0/2] add fuzzing targets for use with OSS-Fuzz

From: Josh Steadmon <steadmon@google.com>

V2 of this series pulls the compiler flags out of the Makefile, to be provided by the user depending on the combination of compiler and fuzzing engine in use. This also makes it more compatible with OSS-Fuzz's build process.

Josh Steadmon (2):
  fuzz: Add basic fuzz testing target.
  fuzz: Add fuzz testing for packfile indices.
 .gitignore          |  3 +++
 Makefile            | 33 +++++++++++++++++++++++++++++++++
 fuzz-pack-headers.c | 14 ++++++++++++++
 fuzz-pack-idx.c     | 13 +++++++++++++
 packfile.c          | 44 +++++++++++++++++++++++++-------------------
 packfile.h          | 13 +++++++++++++
 6 files changed, 101 insertions(+), 19 deletions(-)
 create mode 100644 fuzz-pack-headers.c
 create mode 100644 fuzz-pack-idx.c
Range-diff against v1:
1:  9456c41798 ! 1:  446d8081b1 fuzz: Add basic fuzz testing target.
    @@ -32,6 +32,9 @@
      
     +FUZZ_OBJS += fuzz-pack-headers.o
     +
    ++# Always build fuzz objects even if not testing, to prevent bit-rot.
    ++all:: $(FUZZ_OBJS)
    ++
     +FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))
     +
      # Empty...
    @@ -46,14 +49,13 @@
      	git.o
      ifndef NO_CURL
     @@
    - cocciclean:
    - 	$(RM) contrib/coccinelle/*.cocci.patch*
    - 
    --clean: profile-clean coverage-clean cocciclean
    -+clean: profile-clean coverage-clean cocciclean fuzz-clean
    - 	$(RM) *.res
    - 	$(RM) $(OBJECTS)
      	$(RM) $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
    + 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
    + 	$(RM) $(TEST_PROGRAMS) $(NO_INSTALL)
    ++	$(RM) $(FUZZ_PROGRAMS)
    + 	$(RM) -r bin-wrappers $(dep_dirs)
    + 	$(RM) -r po/build/
    + 	$(RM) *.pyc *.pyo */*.pyc */*.pyo command-list.h $(ETAGS_TARGET) tags cscope*
     @@
      cover_db_html: cover_db
      	cover -report html -outputdir cover_db_html cover_db
    @@ -61,24 +63,24 @@
     +
     +### Fuzz testing
     +#
    -+.PHONY: fuzz-clean fuzz-objs fuzz-compile
    -+
    -+FUZZ_CFLAGS = $(CFLAGS) -fsanitize-coverage=trace-pc-guard -fsanitize=address
    -+FUZZ_LDFLAGS = $(FUZZ_CFLAGS)
    -+
    -+
    -+fuzz-clean:
    -+	$(RM) $(FUZZ_PROGRAMS) $(FUZZ_OBJS)
    -+
    -+fuzz-objs: $(FUZZ_OBJS)
    ++# Building fuzz targets generally requires a special set of compiler flags that
    ++# are not necessarily appropriate for general builds, and that vary greatly
    ++# depending on the compiler version used.
    ++#
    ++# An example command to build against libFuzzer from LLVM 4.0.0:
    ++#
    ++# make CC=clang CXX=clang++ \
    ++#      CFLAGS="-fsanitize-coverage=trace-pc-guard -fsanitize=address" \
    ++#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \
    ++#      fuzz-all
    ++#
    ++.PHONY: fuzz-all
     +
    -+fuzz-compile:
    -+	$(MAKE) CC=clang LD=clang CFLAGS="$(FUZZ_CFLAGS)" \
    -+		LDFLAGS="$(FUZZ_LDFLAGS)" all fuzz-objs
    ++$(FUZZ_PROGRAMS): all
    ++	$(QUIET_LINK)$(CXX) $(CFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \
    ++		$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@
     +
    -+$(FUZZ_PROGRAMS): fuzz-compile
    -+	clang++ $(FUZZ_LDFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) $(XDIFF_OBJS) \
    -+		$(EXTLIBS) git.o $@.o /usr/lib/llvm-4.0/lib/libFuzzer.a -o $@
    ++fuzz-all: $(FUZZ_PROGRAMS)
     
      diff --git a/fuzz-pack-headers.c b/fuzz-pack-headers.c
      new file mode 100644
2:  581eb8f817 ! 2:  c7b5a03d81 fuzz: Add fuzz testing for packfile indices.
    @@ -24,23 +24,8 @@
      FUZZ_OBJS += fuzz-pack-headers.o
     +FUZZ_OBJS += fuzz-pack-idx.o
      
    - FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))
    - 
    -@@
    - 
    - ### Fuzz testing
    - #
    --.PHONY: fuzz-clean fuzz-objs fuzz-compile
    -+.PHONY: fuzz-clean fuzz-objs fuzz-compile fuzz-all
    - 
    - FUZZ_CFLAGS = $(CFLAGS) -fsanitize-coverage=trace-pc-guard -fsanitize=address
    - FUZZ_LDFLAGS = $(FUZZ_CFLAGS)
    -@@
    - $(FUZZ_PROGRAMS): fuzz-compile
    - 	clang++ $(FUZZ_LDFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) $(XDIFF_OBJS) \
    - 		$(EXTLIBS) git.o $@.o /usr/lib/llvm-4.0/lib/libFuzzer.a -o $@
    -+
    -+fuzz-all: $(FUZZ_PROGRAMS)
    + # Always build fuzz objects even if not testing, to prevent bit-rot.
    + all:: $(FUZZ_OBJS)
     
      diff --git a/fuzz-pack-idx.c b/fuzz-pack-idx.c
      new file mode 100644
-- 
2.19.0.605.g01d371f741-goog
steadmon@google.com· Oct 13, 2018, 00:58 UTC · re: steadmon@google.com · lore

[PATCH v2 1/2] fuzz: Add basic fuzz testing target.

From: Josh Steadmon <steadmon@google.com>

fuzz-pack-headers.c provides a fuzzing entry point compatible with libFuzzer (and possibly other fuzzing engines).

Signed-off-by: Josh Steadmon <steadmon@google.com>
---
 .gitignore          |  2 ++
 Makefile            | 32 ++++++++++++++++++++++++++++++++
 fuzz-pack-headers.c | 14 ++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 fuzz-pack-headers.c
Show changes to 3 files +48 −0

.gitignore, Makefile, fuzz-pack-headers.c

diff --git a/.gitignore b/.gitignore
index 9d1363a1eb..87a28b3115 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,5 @@
+/fuzz_corpora
+/fuzz-pack-headers
 /GIT-BUILD-OPTIONS
 /GIT-CFLAGS
 /GIT-LDFLAGS
diff --git a/Makefile b/Makefile
index 13e1c52478..7f5a19b8ab 100644
--- a/Makefile
+++ b/Makefile
@@ -590,6 +590,8 @@ XDIFF_OBJS =
 VCSSVN_OBJS =
 GENERATED_H =
 EXTRA_CPPFLAGS =
+FUZZ_OBJS =
+FUZZ_PROGRAMS =
 LIB_OBJS =
 PROGRAM_OBJS =
 PROGRAMS =
@@ -682,6 +684,13 @@ SCRIPTS = $(SCRIPT_SH_INS) \
 
 ETAGS_TARGET = TAGS
 
+FUZZ_OBJS += fuzz-pack-headers.o
+
+# Always build fuzz objects even if not testing, to prevent bit-rot.
+all:: $(FUZZ_OBJS)
+
+FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))
+
 # Empty...
 EXTRA_PROGRAMS =
 
@@ -2250,6 +2259,7 @@ TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST
 OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
 	$(XDIFF_OBJS) \
 	$(VCSSVN_OBJS) \
+	$(FUZZ_OBJS) \
 	common-main.o \
 	git.o
 ifndef NO_CURL
@@ -2937,6 +2947,7 @@ clean: profile-clean coverage-clean cocciclean
 	$(RM) $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS) $(NO_INSTALL)
+	$(RM) $(FUZZ_PROGRAMS)
 	$(RM) -r bin-wrappers $(dep_dirs)
 	$(RM) -r po/build/
 	$(RM) *.pyc *.pyo */*.pyc */*.pyo command-list.h $(ETAGS_TARGET) tags cscope*
@@ -3061,3 +3072,24 @@ cover_db: coverage-report
 cover_db_html: cover_db
 	cover -report html -outputdir cover_db_html cover_db
 
+
+### Fuzz testing
+#
+# Building fuzz targets generally requires a special set of compiler flags that
+# are not necessarily appropriate for general builds, and that vary greatly
+# depending on the compiler version used.
+#
+# An example command to build against libFuzzer from LLVM 4.0.0:
+#
+# make CC=clang CXX=clang++ \
+#      CFLAGS="-fsanitize-coverage=trace-pc-guard -fsanitize=address" \
+#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \
+#      fuzz-all
+#
+.PHONY: fuzz-all
+
+$(FUZZ_PROGRAMS): all
+	$(QUIET_LINK)$(CXX) $(CFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \
+		$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@
+
+fuzz-all: $(FUZZ_PROGRAMS)
diff --git a/fuzz-pack-headers.c b/fuzz-pack-headers.c
new file mode 100644
index 0000000000..99da1d0fd3
--- /dev/null
+++ b/fuzz-pack-headers.c
@@ -0,0 +1,14 @@
+#include "packfile.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+	enum object_type type;
+	unsigned long len;
+
+	unpack_object_header_buffer((const unsigned char *)data,
+				    (unsigned long)size, &type, &len);
+
+	return 0;
+}
-- 
2.19.0.605.g01d371f741-goog
steadmon@google.com· Oct 13, 2018, 00:58 UTC · re: steadmon@google.com · lore

[PATCH v2 2/2] fuzz: Add fuzz testing for packfile indices.

From: Josh Steadmon <steadmon@google.com>

Breaks the majority of check_packed_git_idx() into a separate function, load_idx(). The latter function operates on arbitrary buffers, which makes it suitable as a fuzzing test target.

Signed-off-by: Josh Steadmon <steadmon@google.com>
---
 .gitignore      |  1 +
 Makefile        |  1 +
 fuzz-pack-idx.c | 13 +++++++++++++
 packfile.c      | 44 +++++++++++++++++++++++++-------------------
 packfile.h      | 13 +++++++++++++
 5 files changed, 53 insertions(+), 19 deletions(-)
 create mode 100644 fuzz-pack-idx.c
Show changes to 5 files +53 −19

.gitignore, Makefile, fuzz-pack-idx.c, packfile.c, packfile.h

diff --git a/.gitignore b/.gitignore
index 87a28b3115..64b3377d40 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,6 @@
 /fuzz_corpora
 /fuzz-pack-headers
+/fuzz-pack-idx
 /GIT-BUILD-OPTIONS
 /GIT-CFLAGS
 /GIT-LDFLAGS
diff --git a/Makefile b/Makefile
index 7f5a19b8ab..1b3d893090 100644
--- a/Makefile
+++ b/Makefile
@@ -685,6 +685,7 @@ SCRIPTS = $(SCRIPT_SH_INS) \
 ETAGS_TARGET = TAGS
 
 FUZZ_OBJS += fuzz-pack-headers.o
+FUZZ_OBJS += fuzz-pack-idx.o
 
 # Always build fuzz objects even if not testing, to prevent bit-rot.
 all:: $(FUZZ_OBJS)
diff --git a/fuzz-pack-idx.c b/fuzz-pack-idx.c
new file mode 100644
index 0000000000..0c3d777aac
--- /dev/null
+++ b/fuzz-pack-idx.c
@@ -0,0 +1,13 @@
+#include "object-store.h"
+#include "packfile.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+	struct packed_git p;
+
+	load_idx("fuzz-input", GIT_SHA1_RAWSZ, (void *)data, size, &p);
+
+	return 0;
+}
diff --git a/packfile.c b/packfile.c
index 841b36182f..86074a76e9 100644
--- a/packfile.c
+++ b/packfile.c
@@ -80,10 +80,8 @@ void pack_report(void)
 static int check_packed_git_idx(const char *path, struct packed_git *p)
 {
 	void *idx_map;
-	struct pack_idx_header *hdr;
 	size_t idx_size;
-	uint32_t version, nr, i, *index;
-	int fd = git_open(path);
+	int fd = git_open(path), ret;
 	struct stat st;
 	const unsigned int hashsz = the_hash_algo->rawsz;
 
@@ -101,16 +99,32 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 	idx_map = xmmap(NULL, idx_size, PROT_READ, MAP_PRIVATE, fd, 0);
 	close(fd);
 
-	hdr = idx_map;
+	ret = load_idx(path, hashsz, idx_map, idx_size, p);
+
+	if (ret)
+		munmap(idx_map, idx_size);
+
+	return ret;
+}
+
+int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
+	     size_t idx_size, struct packed_git *p)
+{
+	struct pack_idx_header *hdr = idx_map;
+	uint32_t version, nr, i, *index;
+
+	if (idx_size < 4 * 256 + hashsz + hashsz)
+		return error("index file %s is too small", path);
+	if (idx_map == NULL)
+		return error("empty data");
+
 	if (hdr->idx_signature == htonl(PACK_IDX_SIGNATURE)) {
 		version = ntohl(hdr->idx_version);
-		if (version < 2 || version > 2) {
-			munmap(idx_map, idx_size);
+		if (version < 2 || version > 2)
 			return error("index file %s is version %"PRIu32
 				     " and is not supported by this binary"
 				     " (try upgrading GIT to a newer version)",
 				     path, version);
-		}
 	} else
 		version = 1;
 
@@ -120,10 +134,8 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 		index += 2;  /* skip index header */
 	for (i = 0; i < 256; i++) {
 		uint32_t n = ntohl(index[i]);
-		if (n < nr) {
-			munmap(idx_map, idx_size);
+		if (n < nr)
 			return error("non-monotonic index %s", path);
-		}
 		nr = n;
 	}
 
@@ -135,10 +147,8 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 		 *  - hash of the packfile
 		 *  - file checksum
 		 */
-		if (idx_size != 4*256 + nr * (hashsz + 4) + hashsz + hashsz) {
-			munmap(idx_map, idx_size);
+		if (idx_size != 4 * 256 + nr * (hashsz + 4) + hashsz + hashsz)
 			return error("wrong index v1 file size in %s", path);
-		}
 	} else if (version == 2) {
 		/*
 		 * Minimum size:
@@ -157,20 +167,16 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 		unsigned long max_size = min_size;
 		if (nr)
 			max_size += (nr - 1)*8;
-		if (idx_size < min_size || idx_size > max_size) {
-			munmap(idx_map, idx_size);
+		if (idx_size < min_size || idx_size > max_size)
 			return error("wrong index v2 file size in %s", path);
-		}
 		if (idx_size != min_size &&
 		    /*
 		     * make sure we can deal with large pack offsets.
 		     * 31-bit signed offset won't be enough, neither
 		     * 32-bit unsigned one will be.
 		     */
-		    (sizeof(off_t) <= 4)) {
-			munmap(idx_map, idx_size);
+		    (sizeof(off_t) <= 4))
 			return error("pack too large for current definition of off_t in %s", path);
-		}
 	}
 
 	p->index_version = version;
diff --git a/packfile.h b/packfile.h
index 442625723d..6c4037605d 100644
--- a/packfile.h
+++ b/packfile.h
@@ -164,4 +164,17 @@ extern int has_pack_index(const unsigned char *sha1);
  */
 extern int is_promisor_object(const struct object_id *oid);
 
+/*
+ * Expose a function for fuzz testing.
+ *
+ * load_idx() parses a block of memory as a packfile index and puts the results
+ * into a struct packed_git.
+ *
+ * This function should not be used directly. It is exposed here only so that we
+ * have a convenient entry-point for fuzz testing. For real uses, you should
+ * probably use open_pack_index() or parse_pack_index() instead.
+ */
+extern int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
+		    size_t idx_size, struct packed_git *p);
+
 #endif
-- 
2.19.0.605.g01d371f741-goog
Junio C Hamano· Oct 16, 2018, 06:18 UTC · re: steadmon@google.com · lore

Re: [PATCH v2 0/2] add fuzzing targets for use with OSS-Fuzz

steadmon@google.com writes:
Show 6 quoted lines
> From: Josh Steadmon <steadmon@google.com>
>
> V2 of this series pulls the compiler flags out of the Makefile, to be
> provided by the user depending on the combination of compiler and
> fuzzing engine in use. This also makes it more compatible with
> OSS-Fuzz's build process.

Thanks, will replace. I think we can merge this to 'next' and down and build incrementally on top.

← back to recent threads