[PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'

[Junio C Hamano <gitster@pobox.com>]

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 <gitster@pobox.com>
---

 * Now with an updated test; while writing it it uncovered a bug in
   the original test that expected to fail---they failed alright but
   sometimes failed for a wrong reason.

 config.c               | 48 +++++++++++++++++++++++++++++++++++++-----------
 t/t1300-repo-config.sh | 16 ++++++++++++++++
 2 files changed, 53 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..ea371020fa 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1143,6 +1143,22 @@ 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
+
+for VAR in a.b a."b c".d
+do
+	test_expect_success "git -c $VAR=VAL works with valid '$VAR'" '
+		echo VAL >expect &&
+		git -c "$VAR=VAL" config --get "$VAR" >actual &&
+		test_cmp expect actual
+	'
+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-222-gff02733afe