All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Patrick Steinhardt <ps@pks.im>, Elijah Newren <newren@gmail.com>,
	Jeff King <peff@peff.net>,
	"brian m . carlson" <sandals@crustytoothpaste.net>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Justin Tobler <jltobler@gmail.com>,
	Christian Couder <christian.couder@gmail.com>
Subject: [PATCH] git: avoid segfault on "git --shallow-file" without a value
Date: Tue, 11 Aug 2026 14:14:46 +0200	[thread overview]
Message-ID: <20260811121446.2080190-1-christian.couder@gmail.com> (raw)

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(-)

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


             reply	other threads:[~2026-08-11 12:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 12:14 Christian Couder [this message]
2026-08-11 19:16 ` [PATCH] git: avoid segfault on "git --shallow-file" without a value Junio C Hamano
2026-08-12 16:15   ` Christian Couder
2026-08-12 17:13     ` Junio C Hamano
2026-08-12 11:22 ` Patrick Steinhardt
2026-08-12 15:42   ` Christian Couder
2026-08-13  7:38     ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260811121446.2080190-1-christian.couder@gmail.com \
    --to=christian.couder@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.