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

From

Junio C Hamano <gitster@pobox.com>

Date

Feb 21, 2017, 21:24 UTC

Message-ID

<xmqqd1ebfd9l.fsf_-_@gitster.mtv.corp.google.com>

In-Reply-To

<xmqqlgszffm0.fsf@gitster.mtv.corp.google.com>

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