{"thread":{"id":"61117","subject":"[PATCH] fuzz: add fuzzer for config parsing","startedAt":"2024-03-14T06:41:21Z","lastAt":"2024-03-15T05:47:35Z","messageCount":3,"participants":["Brian Tracy via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"490611","messageId":"pull.1692.git.1710398478718.gitgitgadget@gmail.com","threadId":"61117","inReplyTo":null,"subject":"[PATCH] fuzz: add fuzzer for config parsing","fromName":"Brian Tracy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-14T06:41:18Z","receivedAt":"2024-03-14T06:41:21Z","isPatch":true,"sender":{"key":"name:Brian Tracy","avatar":null},"body":"From: Brian C Tracy <brian.tracy33@gmail.com>\n\nAdd a new fuzz target that exercises the parsing of git configs.\nThe existing git_config_from_mem function is a perfect entry point\nfor fuzzing as it exercises the same code paths as the rest of the\nconfig parsing functions and offers an easily fuzzable interface.\n\nConfig parsing is a useful thing to fuzz because it operates on user\ncontrolled data and is a central component of many git operations.\n\nSigned-off-by: Brian C Tracy <brian.tracy33@gmail.com>\n---\n    fuzz: add fuzzer for config parsing\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1692%2Fbriantracy%2Fconfig-fuzzer-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1692/briantracy/config-fuzzer-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1692\n\n Makefile                            |  1 +\n ci/run-build-and-minimal-fuzzers.sh |  2 +-\n oss-fuzz/.gitignore                 |  1 +\n oss-fuzz/fuzz-config.c              | 32 +++++++++++++++++++++++++++++\n 4 files changed, 35 insertions(+), 1 deletion(-)\n create mode 100644 oss-fuzz/fuzz-config.c\n\ndiff --git a/Makefile b/Makefile\nindex 4e255c81f22..aa6c852548c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -760,6 +760,7 @@ FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o\n FUZZ_OBJS += oss-fuzz/fuzz-date.o\n FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o\n FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o\n+FUZZ_OBJS += oss-fuzz/fuzz-config.o\n .PHONY: fuzz-objs\n fuzz-objs: $(FUZZ_OBJS)\n \ndiff --git a/ci/run-build-and-minimal-fuzzers.sh b/ci/run-build-and-minimal-fuzzers.sh\nindex 8ba486f6598..29a21281f50 100755\n--- a/ci/run-build-and-minimal-fuzzers.sh\n+++ b/ci/run-build-and-minimal-fuzzers.sh\n@@ -12,7 +12,7 @@ group \"Build fuzzers\" make \\\n \tLIB_FUZZING_ENGINE=\"-fsanitize=fuzzer,address\" \\\n \tfuzz-all\n \n-for fuzzer in commit-graph date pack-headers pack-idx ; do\n+for fuzzer in commit-graph date pack-headers pack-idx config ; do\n \tbegin_group \"fuzz-$fuzzer\"\n \t./oss-fuzz/fuzz-$fuzzer -verbosity=0 -runs=1 || exit 1\n \tend_group \"fuzz-$fuzzer\"\ndiff --git a/oss-fuzz/.gitignore b/oss-fuzz/.gitignore\nindex 5b954088254..892fb09a95d 100644\n--- a/oss-fuzz/.gitignore\n+++ b/oss-fuzz/.gitignore\n@@ -2,3 +2,4 @@ fuzz-commit-graph\n fuzz-date\n fuzz-pack-headers\n fuzz-pack-idx\n+fuzz-config\ndiff --git a/oss-fuzz/fuzz-config.c b/oss-fuzz/fuzz-config.c\nnew file mode 100644\nindex 00000000000..5a1b39aa1e7\n--- /dev/null\n+++ b/oss-fuzz/fuzz-config.c\n@@ -0,0 +1,32 @@\n+#include \"git-compat-util.h\"\n+#include \"config.h\"\n+\n+#include <stdio.h>\n+#include <string.h>\n+\n+int LLVMFuzzerTestOneInput(const uint8_t *, size_t);\n+static int config_parser_callback(const char *, const char *,\n+\t\t\t\t\tconst struct config_context *, void *);\n+\n+static int config_parser_callback(const char *key, const char *value,\n+\t\t\t\t\tconst struct config_context *ctx UNUSED,\n+\t\t\t\t\tvoid *data UNUSED)\n+{\n+\t/* Visit every byte of memory we are given to make sure the parser\n+\t * gave it to us appropriately. Ensure a return of 0 to indicate\n+\t * success so the parsing continues. */\n+\tint c = strlen(key);\n+\tif (value)\n+\t\tc += strlen(value);\n+\treturn c < 0;\n+}\n+\n+int LLVMFuzzerTestOneInput(const uint8_t *data, const size_t size)\n+{\n+\tstruct config_options config_opts = { 0 };\n+\tconfig_opts.error_action = CONFIG_ERROR_SILENT;\n+\tgit_config_from_mem(config_parser_callback, CONFIG_ORIGIN_BLOB,\n+\t\t\t\t\"fuzztest-config\", (const char *)data, size, NULL,\n+\t\t\t\tCONFIG_SCOPE_UNKNOWN, &config_opts);\n+\treturn 0;\n+}\n\nbase-commit: 945115026aa63df4ab849ab14a04da31623abece\n-- \ngitgitgadget\n"},{"id":"490626","messageId":"xmqq34sswyas.fsf@gitster.g","threadId":"61117","inReplyTo":"pull.1692.git.1710398478718.gitgitgadget@gmail.com","subject":"Re: [PATCH] fuzz: add fuzzer for config parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-14T16:52:43Z","receivedAt":"2024-03-14T16:52:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Brian Tracy via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> diff --git a/Makefile b/Makefile\n> index 4e255c81f22..aa6c852548c 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -760,6 +760,7 @@ FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o\n>  FUZZ_OBJS += oss-fuzz/fuzz-date.o\n>  FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o\n>  FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o\n> +FUZZ_OBJS += oss-fuzz/fuzz-config.o\n\nKeep the similar things alphabetically ordered, i.e. \"config\" comes\nafter \"commit-graph\" before \"date\".\n\n> -for fuzzer in commit-graph date pack-headers pack-idx ; do\n> +for fuzzer in commit-graph date pack-headers pack-idx config ; do\n\nDitto.\n\n> diff --git a/oss-fuzz/.gitignore b/oss-fuzz/.gitignore\n> index 5b954088254..892fb09a95d 100644\n> --- a/oss-fuzz/.gitignore\n> +++ b/oss-fuzz/.gitignore\n> @@ -2,3 +2,4 @@ fuzz-commit-graph\n>  fuzz-date\n>  fuzz-pack-headers\n>  fuzz-pack-idx\n> +fuzz-config\n\nDitto.\n\n> diff --git a/oss-fuzz/fuzz-config.c b/oss-fuzz/fuzz-config.c\n> new file mode 100644\n> index 00000000000..5a1b39aa1e7\n> --- /dev/null\n> +++ b/oss-fuzz/fuzz-config.c\n> @@ -0,0 +1,32 @@\n> +#include \"git-compat-util.h\"\n> +#include \"config.h\"\n> +\n> +#include <stdio.h>\n> +#include <string.h>\n\nYou shouldn't have to include system header files yourself.\n\n\"git-compat-util.h\" is there exactly for insulating yor code from\ndetails such as which system headers in what order need to be\nincluded and which CPP feature macros need to be defined before\ntheir inclusion, and including it as the first header file should\nbe sufficient.\n\n> +\n> +int LLVMFuzzerTestOneInput(const uint8_t *, size_t);\n> +static int config_parser_callback(const char *, const char *,\n> +\t\t\t\t\tconst struct config_context *, void *);\n> +\n> +static int config_parser_callback(const char *key, const char *value,\n> +\t\t\t\t\tconst struct config_context *ctx UNUSED,\n> +\t\t\t\t\tvoid *data UNUSED)\n> +{\n> +\t/* Visit every byte of memory we are given to make sure the parser\n> +\t * gave it to us appropriately. Ensure a return of 0 to indicate\n> +\t * success so the parsing continues. */\n\n\t/*\n         * Our multi-line comments are formatted with\n\t * the slash-asterisk at the beginning and\n\t * the asterisk-slash at the end on their\n\t * own lines.\n\t */\n\n> +\tint c = strlen(key);\n\nIsn't the type of return value from strlen() size_t?  It should be\navailable by inclusing \"git-compat-util.h\" to you for free.\n\n> +\tif (value)\n> +\t\tc += strlen(value);\n> +\treturn c < 0;\n\nIs there a reason why this is not \"return 0\"?  Is it to fool a\nclever compiler optimization that knows that omitting calls to\ngit_config_from_mem() is safe if its callback function does not have\nexternally observable side effects and always succeeds?\n\n> +}\n> +\n> +int LLVMFuzzerTestOneInput(const uint8_t *data, const size_t size)\n> +{\n> +\tstruct config_options config_opts = { 0 };\n\nWe still honor -Wdecl-after-statement and leave a blank line between\nthe decl and the first statement here.\n\n> +\tconfig_opts.error_action = CONFIG_ERROR_SILENT;\n> +\tgit_config_from_mem(config_parser_callback, CONFIG_ORIGIN_BLOB,\n> +\t\t\t\t\"fuzztest-config\", (const char *)data, size, NULL,\n> +\t\t\t\tCONFIG_SCOPE_UNKNOWN, &config_opts);\n> +\treturn 0;\n> +}\n>\n> base-commit: 945115026aa63df4ab849ab14a04da31623abece\n\nThanks.\n"},{"id":"490663","messageId":"pull.1692.v2.git.1710481652130.gitgitgadget@gmail.com","threadId":"61117","inReplyTo":"pull.1692.git.1710398478718.gitgitgadget@gmail.com","subject":"[PATCH v2] fuzz: add fuzzer for config parsing","fromName":"Brian Tracy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-15T05:47:31Z","receivedAt":"2024-03-15T05:47:35Z","isPatch":true,"sender":{"key":"name:Brian Tracy","avatar":null},"body":"From: Brian C Tracy <brian.tracy33@gmail.com>\n\nAdd a new fuzz target that exercises the parsing of git configs.\nThe existing git_config_from_mem function is a perfect entry point\nfor fuzzing as it exercises the same code paths as the rest of the\nconfig parsing functions and offers an easily fuzzable interface.\n\nConfig parsing is a useful thing to fuzz because it operates on user\ncontrolled data and is a central component of many git operations.\n\nSigned-off-by: Brian C Tracy <brian.tracy33@gmail.com>\n---\n    fuzz: add fuzzer for config parsing\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1692%2Fbriantracy%2Fconfig-fuzzer-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1692/briantracy/config-fuzzer-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1692\n\nRange-diff vs v1:\n\n 1:  a43a2e3a07c ! 1:  be6aa5efb1c fuzz: add fuzzer for config parsing\n     @@ Commit message\n          Signed-off-by: Brian C Tracy <brian.tracy33@gmail.com>\n      \n       ## Makefile ##\n     -@@ Makefile: FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o\n     +@@ Makefile: ETAGS_TARGET = TAGS\n     + # runs in the future.\n     + FUZZ_OBJS += oss-fuzz/dummy-cmd-main.o\n     + FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o\n     ++FUZZ_OBJS += oss-fuzz/fuzz-config.o\n       FUZZ_OBJS += oss-fuzz/fuzz-date.o\n       FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o\n       FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o\n     -+FUZZ_OBJS += oss-fuzz/fuzz-config.o\n     - .PHONY: fuzz-objs\n     - fuzz-objs: $(FUZZ_OBJS)\n     - \n      \n       ## ci/run-build-and-minimal-fuzzers.sh ##\n      @@ ci/run-build-and-minimal-fuzzers.sh: group \"Build fuzzers\" make \\\n     @@ ci/run-build-and-minimal-fuzzers.sh: group \"Build fuzzers\" make \\\n       \tfuzz-all\n       \n      -for fuzzer in commit-graph date pack-headers pack-idx ; do\n     -+for fuzzer in commit-graph date pack-headers pack-idx config ; do\n     ++for fuzzer in commit-graph config date pack-headers pack-idx ; do\n       \tbegin_group \"fuzz-$fuzzer\"\n       \t./oss-fuzz/fuzz-$fuzzer -verbosity=0 -runs=1 || exit 1\n       \tend_group \"fuzz-$fuzzer\"\n      \n       ## oss-fuzz/.gitignore ##\n     -@@ oss-fuzz/.gitignore: fuzz-commit-graph\n     +@@\n     + fuzz-commit-graph\n     ++fuzz-config\n       fuzz-date\n       fuzz-pack-headers\n       fuzz-pack-idx\n     -+fuzz-config\n      \n       ## oss-fuzz/fuzz-config.c (new) ##\n      @@\n      +#include \"git-compat-util.h\"\n      +#include \"config.h\"\n      +\n     -+#include <stdio.h>\n     -+#include <string.h>\n     -+\n      +int LLVMFuzzerTestOneInput(const uint8_t *, size_t);\n      +static int config_parser_callback(const char *, const char *,\n      +\t\t\t\t\tconst struct config_context *, void *);\n     @@ oss-fuzz/fuzz-config.c (new)\n      +\t\t\t\t\tconst struct config_context *ctx UNUSED,\n      +\t\t\t\t\tvoid *data UNUSED)\n      +{\n     -+\t/* Visit every byte of memory we are given to make sure the parser\n     -+\t * gave it to us appropriately. Ensure a return of 0 to indicate\n     -+\t * success so the parsing continues. */\n     -+\tint c = strlen(key);\n     ++\t/*\n     ++\t * Visit every byte of memory we are given to make sure the parser\n     ++\t * gave it to us appropriately. We need to unconditionally return 0,\n     ++\t * but we also want to prevent the strlen from being optimized away.\n     ++\t */\n     ++\tsize_t c = strlen(key);\n     ++\n      +\tif (value)\n      +\t\tc += strlen(value);\n     -+\treturn c < 0;\n     ++\treturn c == SIZE_MAX;\n      +}\n      +\n      +int LLVMFuzzerTestOneInput(const uint8_t *data, const size_t size)\n      +{\n      +\tstruct config_options config_opts = { 0 };\n     ++\n      +\tconfig_opts.error_action = CONFIG_ERROR_SILENT;\n      +\tgit_config_from_mem(config_parser_callback, CONFIG_ORIGIN_BLOB,\n      +\t\t\t\t\"fuzztest-config\", (const char *)data, size, NULL,\n\n\n Makefile                            |  1 +\n ci/run-build-and-minimal-fuzzers.sh |  2 +-\n oss-fuzz/.gitignore                 |  1 +\n oss-fuzz/fuzz-config.c              | 33 +++++++++++++++++++++++++++++\n 4 files changed, 36 insertions(+), 1 deletion(-)\n create mode 100644 oss-fuzz/fuzz-config.c\n\ndiff --git a/Makefile b/Makefile\nindex 4e255c81f22..af32028b18f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -757,6 +757,7 @@ ETAGS_TARGET = TAGS\n # runs in the future.\n FUZZ_OBJS += oss-fuzz/dummy-cmd-main.o\n FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o\n+FUZZ_OBJS += oss-fuzz/fuzz-config.o\n FUZZ_OBJS += oss-fuzz/fuzz-date.o\n FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o\n FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o\ndiff --git a/ci/run-build-and-minimal-fuzzers.sh b/ci/run-build-and-minimal-fuzzers.sh\nindex 8ba486f6598..a51076d18df 100755\n--- a/ci/run-build-and-minimal-fuzzers.sh\n+++ b/ci/run-build-and-minimal-fuzzers.sh\n@@ -12,7 +12,7 @@ group \"Build fuzzers\" make \\\n \tLIB_FUZZING_ENGINE=\"-fsanitize=fuzzer,address\" \\\n \tfuzz-all\n \n-for fuzzer in commit-graph date pack-headers pack-idx ; do\n+for fuzzer in commit-graph config date pack-headers pack-idx ; do\n \tbegin_group \"fuzz-$fuzzer\"\n \t./oss-fuzz/fuzz-$fuzzer -verbosity=0 -runs=1 || exit 1\n \tend_group \"fuzz-$fuzzer\"\ndiff --git a/oss-fuzz/.gitignore b/oss-fuzz/.gitignore\nindex 5b954088254..a877c11f42b 100644\n--- a/oss-fuzz/.gitignore\n+++ b/oss-fuzz/.gitignore\n@@ -1,4 +1,5 @@\n fuzz-commit-graph\n+fuzz-config\n fuzz-date\n fuzz-pack-headers\n fuzz-pack-idx\ndiff --git a/oss-fuzz/fuzz-config.c b/oss-fuzz/fuzz-config.c\nnew file mode 100644\nindex 00000000000..94027f5b97e\n--- /dev/null\n+++ b/oss-fuzz/fuzz-config.c\n@@ -0,0 +1,33 @@\n+#include \"git-compat-util.h\"\n+#include \"config.h\"\n+\n+int LLVMFuzzerTestOneInput(const uint8_t *, size_t);\n+static int config_parser_callback(const char *, const char *,\n+\t\t\t\t\tconst struct config_context *, void *);\n+\n+static int config_parser_callback(const char *key, const char *value,\n+\t\t\t\t\tconst struct config_context *ctx UNUSED,\n+\t\t\t\t\tvoid *data UNUSED)\n+{\n+\t/*\n+\t * Visit every byte of memory we are given to make sure the parser\n+\t * gave it to us appropriately. We need to unconditionally return 0,\n+\t * but we also want to prevent the strlen from being optimized away.\n+\t */\n+\tsize_t c = strlen(key);\n+\n+\tif (value)\n+\t\tc += strlen(value);\n+\treturn c == SIZE_MAX;\n+}\n+\n+int LLVMFuzzerTestOneInput(const uint8_t *data, const size_t size)\n+{\n+\tstruct config_options config_opts = { 0 };\n+\n+\tconfig_opts.error_action = CONFIG_ERROR_SILENT;\n+\tgit_config_from_mem(config_parser_callback, CONFIG_ORIGIN_BLOB,\n+\t\t\t\t\"fuzztest-config\", (const char *)data, size, NULL,\n+\t\t\t\tCONFIG_SCOPE_UNKNOWN, &config_opts);\n+\treturn 0;\n+}\n\nbase-commit: 945115026aa63df4ab849ab14a04da31623abece\n-- \ngitgitgadget\n"}]}