[PATCH] builtin-config: Fix crash when using "-f <relative path>" from non-root dir

Johan Herland· Jan 26, 2010, 15:02 UTC · lore

When your current directory is not at the root of the working tree, and you use the "-f" option with a relative path, the current code segfaults since argv has already been parsed, and argv[2] is now is a NULL pointer. This patch replaces the incorrect argv[2] with the variable holding the given config file name.

The bug was introduced by d64ec16... (git config: reorganize to use parseopt).

Signed-off-by: Johan Herland <johan@herland.net>
---

This is probably v1.7.0 material, and maybe even suitable for 'maint'. AFAICS, d64ec16... has been with us since before v1.6.3-rc0.

...Johan

 builtin-config.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to builtin-config.c +1 −1

diff --git a/builtin-config.c b/builtin-config.c
index 2e3ef91..4bc46b1 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -358,7 +358,7 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
 		if (!is_absolute_path(given_config_file) && prefix)
 			config_exclusive_filename = prefix_filename(prefix,
 								    strlen(prefix),
-								    argv[2]);
+								    given_config_file);
 		else
 			config_exclusive_filename = given_config_file;
 	}

-- 
1.6.6.rc2.5.g49666
-- 
Johan Herland, <johan@herland.net>
www.herland.net

Junio C Hamano· Jan 26, 2010, 20:02 UTC · re: Johan Herland · lore

Re: [PATCH] builtin-config: Fix crash when using "-f <relative path>" from non-root dir

Johan Herland <johan@herland.net> writes:

> When your current directory is not at the root of the working tree, and you
> use the "-f" option with a relative path, the current code segfaults since
> argv has already been parsed, and argv[2] is now is a NULL pointer.

Strictly speaking, the last sentence is not necessarily true. argv[2] is typically beyond the NULL pointer parseopt added to terminate the resulting list. E.g. with the attached patch to t1300 (which I will squash in), argv[0] is ein.bahn, argv[1] is NULL (end of argument list after parseopt stripped the options from it), and argv[2] is --file.

Show 10 quoted lines

> This patch replaces the incorrect argv[2] with the variable holding the
> given config file name.
>
> The bug was introduced by d64ec16 (git config: reorganize to use parseopt).
>
> Signed-off-by: Johan Herland <johan@herland.net>
> ---
>
> This is probably v1.7.0 material, and maybe even suitable for 'maint'.
> AFAICS, d64ec16... has been with us since before v1.6.3-rc0.

Yup, this is maint worthy. Even if argv[2] happens to be non-NULL, it certainly is not given_config_file at all.

Thanks.

 t/t1300-repo-config.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

Show changes to t/t1300-repo-config.sh +11 −0

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 43ea283..c81ccf2 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -398,6 +398,17 @@ test_expect_success 'alternative GIT_CONFIG' 'cmp output expect'
 test_expect_success 'alternative GIT_CONFIG (--file)' \
 	'git config --file other-config -l > output && cmp output expect'
 
+test_expect_success 'refer config from subdirectory' '
+	mkdir x &&
+	(
+		cd x &&
+		echo strasse >expect
+		git config --get --file ../other-config ein.bahn >actual &&
+		test_cmp expect actual
+	)
+
+'
+
 GIT_CONFIG=other-config git config anwohner.park ausweis
 
 cat > expect << EOF

patchbuiltin-config: Fix crash when using "-f <relative path>" from non-root dir