diff mbox series

fuzz: add fuzzer for config parsing

Message ID pull.1692.git.1710398478718.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series fuzz: add fuzzer for config parsing | expand

Commit Message

Brian C Tracy March 14, 2024, 6:41 a.m. UTC
From: Brian C Tracy <brian.tracy33@gmail.com>

Add a new fuzz target that exercises the parsing of git configs.
The existing git_config_from_mem function is a perfect entry point
for fuzzing as it exercises the same code paths as the rest of the
config parsing functions and offers an easily fuzzable interface.

Config parsing is a useful thing to fuzz because it operates on user
controlled data and is a central component of many git operations.

Signed-off-by: Brian C Tracy <brian.tracy33@gmail.com>
---
    fuzz: add fuzzer for config parsing

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1692%2Fbriantracy%2Fconfig-fuzzer-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1692/briantracy/config-fuzzer-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1692

 Makefile                            |  1 +
 ci/run-build-and-minimal-fuzzers.sh |  2 +-
 oss-fuzz/.gitignore                 |  1 +
 oss-fuzz/fuzz-config.c              | 32 +++++++++++++++++++++++++++++
 4 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 oss-fuzz/fuzz-config.c


base-commit: 945115026aa63df4ab849ab14a04da31623abece

Comments

Junio C Hamano March 14, 2024, 4:52 p.m. UTC | #1
"Brian Tracy via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/Makefile b/Makefile
> index 4e255c81f22..aa6c852548c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -760,6 +760,7 @@ FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o
>  FUZZ_OBJS += oss-fuzz/fuzz-date.o
>  FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o
>  FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o
> +FUZZ_OBJS += oss-fuzz/fuzz-config.o

Keep the similar things alphabetically ordered, i.e. "config" comes
after "commit-graph" before "date".

> -for fuzzer in commit-graph date pack-headers pack-idx ; do
> +for fuzzer in commit-graph date pack-headers pack-idx config ; do

Ditto.

> diff --git a/oss-fuzz/.gitignore b/oss-fuzz/.gitignore
> index 5b954088254..892fb09a95d 100644
> --- a/oss-fuzz/.gitignore
> +++ b/oss-fuzz/.gitignore
> @@ -2,3 +2,4 @@ fuzz-commit-graph
>  fuzz-date
>  fuzz-pack-headers
>  fuzz-pack-idx
> +fuzz-config

Ditto.

> diff --git a/oss-fuzz/fuzz-config.c b/oss-fuzz/fuzz-config.c
> new file mode 100644
> index 00000000000..5a1b39aa1e7
> --- /dev/null
> +++ b/oss-fuzz/fuzz-config.c
> @@ -0,0 +1,32 @@
> +#include "git-compat-util.h"
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <string.h>

You shouldn't have to include system header files yourself.

"git-compat-util.h" is there exactly for insulating yor code from
details such as which system headers in what order need to be
included and which CPP feature macros need to be defined before
their inclusion, and including it as the first header file should
be sufficient.

> +
> +int LLVMFuzzerTestOneInput(const uint8_t *, size_t);
> +static int config_parser_callback(const char *, const char *,
> +					const struct config_context *, void *);
> +
> +static int config_parser_callback(const char *key, const char *value,
> +					const struct config_context *ctx UNUSED,
> +					void *data UNUSED)
> +{
> +	/* Visit every byte of memory we are given to make sure the parser
> +	 * gave it to us appropriately. Ensure a return of 0 to indicate
> +	 * success so the parsing continues. */

	/*
         * Our multi-line comments are formatted with
	 * the slash-asterisk at the beginning and
	 * the asterisk-slash at the end on their
	 * own lines.
	 */

> +	int c = strlen(key);

Isn't the type of return value from strlen() size_t?  It should be
available by inclusing "git-compat-util.h" to you for free.

> +	if (value)
> +		c += strlen(value);
> +	return c < 0;

Is there a reason why this is not "return 0"?  Is it to fool a
clever compiler optimization that knows that omitting calls to
git_config_from_mem() is safe if its callback function does not have
externally observable side effects and always succeeds?

> +}
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, const size_t size)
> +{
> +	struct config_options config_opts = { 0 };

We still honor -Wdecl-after-statement and leave a blank line between
the decl and the first statement here.

> +	config_opts.error_action = CONFIG_ERROR_SILENT;
> +	git_config_from_mem(config_parser_callback, CONFIG_ORIGIN_BLOB,
> +				"fuzztest-config", (const char *)data, size, NULL,
> +				CONFIG_SCOPE_UNKNOWN, &config_opts);
> +	return 0;
> +}
>
> base-commit: 945115026aa63df4ab849ab14a04da31623abece

Thanks.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 4e255c81f22..aa6c852548c 100644
--- a/Makefile
+++ b/Makefile
@@ -760,6 +760,7 @@  FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o
 FUZZ_OBJS += oss-fuzz/fuzz-date.o
 FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o
 FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o
+FUZZ_OBJS += oss-fuzz/fuzz-config.o
 .PHONY: fuzz-objs
 fuzz-objs: $(FUZZ_OBJS)
 
diff --git a/ci/run-build-and-minimal-fuzzers.sh b/ci/run-build-and-minimal-fuzzers.sh
index 8ba486f6598..29a21281f50 100755
--- a/ci/run-build-and-minimal-fuzzers.sh
+++ b/ci/run-build-and-minimal-fuzzers.sh
@@ -12,7 +12,7 @@  group "Build fuzzers" make \
 	LIB_FUZZING_ENGINE="-fsanitize=fuzzer,address" \
 	fuzz-all
 
-for fuzzer in commit-graph date pack-headers pack-idx ; do
+for fuzzer in commit-graph date pack-headers pack-idx config ; do
 	begin_group "fuzz-$fuzzer"
 	./oss-fuzz/fuzz-$fuzzer -verbosity=0 -runs=1 || exit 1
 	end_group "fuzz-$fuzzer"
diff --git a/oss-fuzz/.gitignore b/oss-fuzz/.gitignore
index 5b954088254..892fb09a95d 100644
--- a/oss-fuzz/.gitignore
+++ b/oss-fuzz/.gitignore
@@ -2,3 +2,4 @@  fuzz-commit-graph
 fuzz-date
 fuzz-pack-headers
 fuzz-pack-idx
+fuzz-config
diff --git a/oss-fuzz/fuzz-config.c b/oss-fuzz/fuzz-config.c
new file mode 100644
index 00000000000..5a1b39aa1e7
--- /dev/null
+++ b/oss-fuzz/fuzz-config.c
@@ -0,0 +1,32 @@ 
+#include "git-compat-util.h"
+#include "config.h"
+
+#include <stdio.h>
+#include <string.h>
+
+int LLVMFuzzerTestOneInput(const uint8_t *, size_t);
+static int config_parser_callback(const char *, const char *,
+					const struct config_context *, void *);
+
+static int config_parser_callback(const char *key, const char *value,
+					const struct config_context *ctx UNUSED,
+					void *data UNUSED)
+{
+	/* Visit every byte of memory we are given to make sure the parser
+	 * gave it to us appropriately. Ensure a return of 0 to indicate
+	 * success so the parsing continues. */
+	int c = strlen(key);
+	if (value)
+		c += strlen(value);
+	return c < 0;
+}
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, const size_t size)
+{
+	struct config_options config_opts = { 0 };
+	config_opts.error_action = CONFIG_ERROR_SILENT;
+	git_config_from_mem(config_parser_callback, CONFIG_ORIGIN_BLOB,
+				"fuzztest-config", (const char *)data, size, NULL,
+				CONFIG_SCOPE_UNKNOWN, &config_opts);
+	return 0;
+}