[PATCH] git: avoid segfault on "git --shallow-file" without a value

Christian Couder <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <[email protected]>
In "git.c", the other `handle_options()` options that take their value
as a separate argument, like `--git-dir`, `--namespace` or `-C`, check
that such an argument actually exists before using it, and error out
with a message and the usage string otherwise.

The `--shallow-file` option doesn't perform that check. It blindly
advances past the option and then dereferences the next element of
`argv`, which is the NULL terminator when no value was given. So
`git --shallow-file` segfaults:

  $ git --shallow-file
  Segmentation fault (core dumped)

Let's fix that by checking that a value was given, in the same way and
with a message worded like the ones the other options use.

While at it, let's also set the environment variable before advancing
past the option, instead of advancing first and using `(*argv)[0]`, so
that this option looks like the other ones.

Note that all the in-tree callers passing `--shallow-file` to a `git`
subprocess always pass a value after it, so they are not affected. In
`upload-pack.c` that value is an empty string, which is still accepted.

Signed-off-by: Christian Couder <[email protected]>
---

While working on modernizing `git fast-import`, I noticed that
`--shallow-file` was handled differently than the other options that
take an argument in "git.c", and found this segfault.

I have started working on a better way to handle such options not only
in "git.c" but also in other files. For now though, I think a small
localized bugfix like this is the simplest solution.

Not sure if "t0041-usage.sh" is the best place for testing this, but I
couldn't find a dedicated one.

CI tests all pass, see:

https://github.com/chriscool/git/actions/runs/31478034826

 git.c            | 10 +++++++---
 t/t0041-usage.sh |  7 +++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/git.c b/git.c
index e5f1811b6b..96df15b5cd 100644
--- a/git.c
+++ b/git.c
@@ -304,11 +304,15 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			if (envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--shallow-file")) {
-			(*argv)++;
-			(*argc)--;
-			setenv(GIT_SHALLOW_FILE_ENVIRONMENT, (*argv)[0], 1);
+			if (*argc < 2) {
+				fprintf(stderr, _("no file given for '%s' option\n" ), "--shallow-file");
+				usage(git_usage_string);
+			}
+			setenv(GIT_SHALLOW_FILE_ENVIRONMENT, (*argv)[1], 1);
 			if (envchanged)
 				*envchanged = 1;
+			(*argv)++;
+			(*argc)--;
 		} else if (!strcmp(cmd, "-C")) {
 			if (*argc < 2) {
 				fprintf(stderr, _("no directory given for '%s' option\n" ), "-C");
diff --git a/t/t0041-usage.sh b/t/t0041-usage.sh
index 51af7cc030..2a9c5eafca 100755
--- a/t/t0041-usage.sh
+++ b/t/t0041-usage.sh
@@ -107,4 +107,11 @@ test_expect_success 'for-each-ref usage error' '
 	test_grep "usage" actual.err
 '
 
+test_expect_success 'git --shallow-file without a value' '
+	test_must_fail git --shallow-file >actual 2>actual.err &&
+	test_line_count = 0 actual &&
+	test_grep "no file given for " actual.err &&
+	test_grep "usage" actual.err
+'
+
 test_done
-- 
2.55.0.530.gdb3615d990.dirty
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.