The parsing of one-shot assignments of configuration variables that
come from the command line historically was quite loose and allowed
anything to pass.
The configuration variable names that come from files are validated
in git_config_parse_source(), which uses get_base_var() that grabs
the <section> (and subsection) while making sure that <section>
consists of iskeychar() letters, the function itself that makes sure
that the first letter in <variable> is isalpha(), and get_value()
that grabs the remainder of the <variable> name while making sure
that it consists of iskeychar() letters.
Perform an equivalent check in canonicalize_config_variable_name()
to catch invalid configuration variable names that come from the
command line.
Signed-off-by: Junio C Hamano <gits...@pobox.com>
---
Junio C Hamano <gits...@pobox.com> writes:
> One thing I noticed is that "git config --get X" will correctly
> diagnose that a dot-less X is not a valid variable name, but we do
> not seem to diagnose "git -c X=V <cmd>" as invalid.
>
> Perhaps we should, but it is not the focus of this topic.
And here is a follow-up on that tangent.
config.c | 48 +++++++++++++++++++++++++++++++++++++-----------
t/t1300-repo-config.sh | 7 +++++++
2 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/config.c b/config.c
index 4128debc71..e7f7ff1938 100644
--- a/config.c
+++ b/config.c
@@ -199,32 +199,62 @@ void git_config_push_parameter(const char *text)
strbuf_release(&env);
}
+static inline int iskeychar(int c)
+{
+ return isalnum(c) || c == '-';
+}
+
/*
* downcase the <section> and <variable> in <section>.<variable> or
* <section>.<subsection>.<variable> and do so in place. <subsection>
* is left intact.
+ *
+ * The configuration variable names that come from files are validated
+ * in git_config_parse_source(), which uses get_base_var() that grabs
+ * the <section> (and subsection) while making sure that <section>
+ * consists of iskeychar() letters, the function itself that makes
+ * sure that the first letter in <variable> is isalpha(), and
+ * get_value() that grabs the remainder of the <variable> name while
+ * making sure that it consists of iskeychar() letters. Perform a
+ * matching validation for configuration variables that come from
+ * the command line.
*/
-static void canonicalize_config_variable_name(char *varname)
+static int canonicalize_config_variable_name(char *varname)
{
- char *cp, *last_dot;
+ char *cp, *first_dot, *last_dot;
/* downcase the first segment */
for (cp = varname; *cp; cp++) {
if (*cp == '.')
break;
+ if (!iskeychar(*cp))
+ return -1;
*cp = tolower(*cp);
}
if (!*cp)
- return;
+ return -1; /* no dot anywhere? */
+
+ first_dot = cp;
+ if (first_dot == varname)
+ return -1; /* no section? */
/* find the last dot (we start from the first dot we just found) */
- for (last_dot = cp; *cp; cp++)
+ for (; *cp; cp++)
if (*cp == '.')
last_dot = cp;
+ if (!last_dot[1])
+ return -1; /* no variable? */
+
/* downcase the last segment */
- for (cp = last_dot; *cp; cp++)
+ for (cp = last_dot + 1; *cp; cp++) {
+ if (cp == last_dot + 1 && !isalpha(*cp))
+ return -1;
+ else if (!iskeychar(*cp))
+ return -1;
*cp = tolower(*cp);
+ }
+ return 0;
}
int git_config_parse_parameter(const char *text,
@@ -249,7 +279,8 @@ int git_config_parse_parameter(const char *text,
strbuf_list_free(pair);
return error("bogus config parameter: %s", text);
}
- canonicalize_config_variable_name(pair[0]->buf);
+ if (canonicalize_config_variable_name(pair[0]->buf))
+ return error("bogus config parameter: %s", text);
if (fn(pair[0]->buf, value, data) < 0) {
strbuf_list_free(pair);
return -1;
@@ -382,11 +413,6 @@ static char *parse_value(void)
}
}
-static inline int iskeychar(int c)
-{
- return isalnum(c) || c == '-';
-}
-
static int get_value(config_fn_t fn, void *data, struct strbuf *name)
{
int c;
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 7a16f66a9d..92a5d0dfb1 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1143,6 +1143,13 @@ test_expect_success 'last one wins: three level vars' '
test_cmp expect actual
'
+for VAR in a .a a. a.0b a."b c". a."b c".0d
+do
+ test_expect_success "git -c $VAR=VAL rejects invalid '$VAR'" '
+ test_must_fail git -c $VAR=VAL config -l
+ '
+done
+
test_expect_success 'git -c is not confused by empty environment' '
GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list
'
--
2.12.0-rc2-221-ga92f6f5816
