diff mbox series

[1/3] fuzz: port fuzz-credential-from-url-gently from OSS-Fuzz

Message ID 625b8d607ed2c95e396e7794616d9f290f23d15c.1728594659.git.steadmon@google.com (mailing list archive)
State Superseded
Headers show
Series fuzz: port OSS-Fuzz tests back to Git | expand

Commit Message

Josh Steadmon Oct. 10, 2024, 9:11 p.m. UTC
From: Eric Sesterhenn <eric.sesterhenn@x41-dsec.de>

Git's fuzz tests are run continuously as part of OSS-Fuzz [1]. Several
additional fuzz tests have been contributed directly to OSS-Fuzz;
however, these tests are vulnerable to bitrot because they are not built
during Git's CI runs, and thus breaking changes are much less likely to
be noticed by Git contributors.

Port one of these tests back to the Git project:
fuzz-credential-from-url-gently

This test was originally written by Eric Sesterhenn as part of a
security audit of Git [2]. It was then contributed to the OSS-Fuzz repo
in commit c58ac4492 (Git fuzzing: uncomment the existing and add new
targets. (#11486), 2024-02-21) by Jaroslav Lobačevski. I (Josh Steadmon)
have verified with both Eric and Jaroslav that they're OK with moving
this test to the Git project.

[1] https://github.com/google/oss-fuzz
[2] https://ostif.org/wp-content/uploads/2023/01/X41-OSTIF-Gitlab-Git-Security-Audit-20230117-public.pdf

Co-authored-by: Jaroslav Lobačevski <jarlob@gmail.com>
Co-authored-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
 Makefile                                   |  1 +
 ci/run-build-and-minimal-fuzzers.sh        | 13 +++++++--
 oss-fuzz/.gitignore                        |  1 +
 oss-fuzz/fuzz-credential-from-url-gently.c | 32 ++++++++++++++++++++++
 4 files changed, 45 insertions(+), 2 deletions(-)
 create mode 100644 oss-fuzz/fuzz-credential-from-url-gently.c

Comments

Oswald Buddenhagen Oct. 11, 2024, 9:13 a.m. UTC | #1
just some nits:

On Thu, Oct 10, 2024 at 02:11:53PM -0700, Josh Steadmon wrote:
>+++ b/ci/run-build-and-minimal-fuzzers.sh
>+fuzzers="
>+commit-graph \
>+config \
>+credential-from-url-gently \
>+date \
>+pack-headers \
>+pack-idx \
>+"
the trailing space-backslashes can be left out, which would make the
code less noisy. then the variable would contain linebreaks instead of
spaces, which the for loop would be just fine with (as $IFS contains
LF).

>+for fuzzer in $fuzzers ; do
>
the space before the semicolon seems excessive.

>+++ b/oss-fuzz/fuzz-credential-from-url-gently.c
>+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
>+
are these seemingly redundant prototypes meant to suppress compiler
warnings?

>+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
>+{
Junio C Hamano Oct. 11, 2024, 4:35 p.m. UTC | #2
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:

> just some nits:
>
> On Thu, Oct 10, 2024 at 02:11:53PM -0700, Josh Steadmon wrote:
>>+++ b/ci/run-build-and-minimal-fuzzers.sh
>>+fuzzers="
>>+commit-graph \
>>...
>>+pack-idx \
>>+"
> the trailing space-backslashes can be left out, which would make the
> code less noisy. then the variable would contain linebreaks instead of
> spaces, which the for loop would be just fine with (as $IFS contains
> LF).

Perfect.

Thanks.
Josh Steadmon Oct. 14, 2024, 8:35 p.m. UTC | #3
On 2024.10.11 11:13, Oswald Buddenhagen wrote:
> just some nits:
> 
> On Thu, Oct 10, 2024 at 02:11:53PM -0700, Josh Steadmon wrote:
> > +++ b/ci/run-build-and-minimal-fuzzers.sh
> > +fuzzers="
> > +commit-graph \
> > +config \
> > +credential-from-url-gently \
> > +date \
> > +pack-headers \
> > +pack-idx \
> > +"
> the trailing space-backslashes can be left out, which would make the
> code less noisy. then the variable would contain linebreaks instead of
> spaces, which the for loop would be just fine with (as $IFS contains
> LF).

Fixed in V2.

> > +for fuzzer in $fuzzers ; do
> > 
> the space before the semicolon seems excessive.

Fixed in V2.

> > +++ b/oss-fuzz/fuzz-credential-from-url-gently.c
> > +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
> > +
> are these seemingly redundant prototypes meant to suppress compiler
> warnings?

Yes, unfortunately we get complaints without them.

> > +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> > +{

Thanks for the review!
Josh Steadmon Oct. 14, 2024, 8:43 p.m. UTC | #4
On 2024.10.10 14:11, Josh Steadmon wrote:
> diff --git a/ci/run-build-and-minimal-fuzzers.sh b/ci/run-build-and-minimal-fuzzers.sh
> index af8065f349..d9d3ad23c7 100755
> --- a/ci/run-build-and-minimal-fuzzers.sh
> +++ b/ci/run-build-and-minimal-fuzzers.sh
> @@ -13,8 +13,17 @@ group "Build fuzzers" make \
>  	LIB_FUZZING_ENGINE="-fsanitize=fuzzer,address" \
>  	fuzz-all
>  
> -for fuzzer in commit-graph config date pack-headers pack-idx ; do
> +fuzzers="
> +commit-graph \
> +config \
> +credential-from-url-gently \
> +date \
> +pack-headers \
> +pack-idx \
> +"
> +
> +for fuzzer in $fuzzers ; do
>  	begin_group "fuzz-$fuzzer"
> -	./oss-fuzz/fuzz-$fuzzer -verbosity=0 -runs=1 || exit 1
> +	echo ./oss-fuzz/fuzz-$fuzzer -verbosity=0 -runs=1 || exit 1
>  	end_group "fuzz-$fuzzer"
>  done

I'm not sure how this "echo" got into the diff here, but I've removed it
in V2.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index e298c8b55e..3ce391062f 100644
--- a/Makefile
+++ b/Makefile
@@ -2378,6 +2378,7 @@  endif
 FUZZ_OBJS += oss-fuzz/dummy-cmd-main.o
 FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o
 FUZZ_OBJS += oss-fuzz/fuzz-config.o
+FUZZ_OBJS += oss-fuzz/fuzz-credential-from-url-gently.o
 FUZZ_OBJS += oss-fuzz/fuzz-date.o
 FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o
 FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o
diff --git a/ci/run-build-and-minimal-fuzzers.sh b/ci/run-build-and-minimal-fuzzers.sh
index af8065f349..d9d3ad23c7 100755
--- a/ci/run-build-and-minimal-fuzzers.sh
+++ b/ci/run-build-and-minimal-fuzzers.sh
@@ -13,8 +13,17 @@  group "Build fuzzers" make \
 	LIB_FUZZING_ENGINE="-fsanitize=fuzzer,address" \
 	fuzz-all
 
-for fuzzer in commit-graph config date pack-headers pack-idx ; do
+fuzzers="
+commit-graph \
+config \
+credential-from-url-gently \
+date \
+pack-headers \
+pack-idx \
+"
+
+for fuzzer in $fuzzers ; do
 	begin_group "fuzz-$fuzzer"
-	./oss-fuzz/fuzz-$fuzzer -verbosity=0 -runs=1 || exit 1
+	echo ./oss-fuzz/fuzz-$fuzzer -verbosity=0 -runs=1 || exit 1
 	end_group "fuzz-$fuzzer"
 done
diff --git a/oss-fuzz/.gitignore b/oss-fuzz/.gitignore
index a877c11f42..2cfc845b20 100644
--- a/oss-fuzz/.gitignore
+++ b/oss-fuzz/.gitignore
@@ -1,5 +1,6 @@ 
 fuzz-commit-graph
 fuzz-config
+fuzz-credential-from-url-gently
 fuzz-date
 fuzz-pack-headers
 fuzz-pack-idx
diff --git a/oss-fuzz/fuzz-credential-from-url-gently.c b/oss-fuzz/fuzz-credential-from-url-gently.c
new file mode 100644
index 0000000000..c872f9ad2d
--- /dev/null
+++ b/oss-fuzz/fuzz-credential-from-url-gently.c
@@ -0,0 +1,32 @@ 
+#include "git-compat-util.h"
+#include <stddef.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <stdio.h>
+#include "credential.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+	struct credential c;
+	char *buf;
+
+	buf = malloc(size + 1);
+	if (!buf)
+		return 0;
+
+	memcpy(buf, data, size);
+	buf[size] = 0;
+
+	// start fuzzing
+	credential_init(&c);
+	credential_from_url_gently(&c, buf, 1);
+
+	// cleanup
+	credential_clear(&c);
+	free(buf);
+
+	return 0;
+}