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 <christian.couder@gmail.com> ---
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(-)
Show changes to 2 files +14 −3
git.c, t/t0041-usage.sh
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