config: validate quoted section value

When we reach a whitespace after a section name, we assume that what
will follow will be a quoted subsection name.  Pass the current position
of the line being parsed to the subsection parser, so that it can
validate that subsequent characters are additional whitespace or a
single quote.

Previously we would begin parsing after the section name, looking for
the first quotation mark.  This allows invalid characters to embed
themselves between the end of the section name and the first quotation
mark, eg `[section foo "subsection"]`, which is illegal.

2 files changed, 79 insertions, 10 deletions

diff --git a/src/config_parse.c b/src/config_parse.c
index 6b162cbef..6e9b9a805 100644
--- a/src/config_parse.c
+++ b/src/config_parse.c
@@ -66,26 +66,25 @@ static int strip_comments(char *line, int in_quotes)
 }
 
 
-static int parse_section_header_ext(git_config_parser *reader, const char *line, const char *base_name, char **section_name)
+static int parse_section_header_ext(git_config_parser *reader, const char *line, size_t pos, const char *base_name, char **section_name)
 {
 	int c, rpos;
-	char *first_quote, *last_quote;
+	const char *first_quote, *last_quote;
 	const char *line_start = line;
 	git_buf buf = GIT_BUF_INIT;
 	size_t quoted_len, alloc_len, base_name_len = strlen(base_name);
 
-	/*
-	 * base_name is what came before the space. We should be at the
-	 * first quotation mark, except for now, line isn't being kept in
-	 * sync so we only really use it to calculate the length.
-	 */
+	/* Skip any additional whitespace before our section name */
+	while (git__isspace(line[pos]))
+		pos++;
 
-	first_quote = strchr(line, '"');
-	if (first_quote == NULL) {
+	/* We should be at the first quotation mark. */
+	if (line[pos] != '"') {
 		set_parse_error(reader, 0, "missing quotation marks in section header");
 		goto end_error;
 	}
 
+	first_quote = &line[pos];
 	last_quote = strrchr(line, '"');
 	quoted_len = last_quote - first_quote;
 
@@ -192,7 +191,7 @@ static int parse_section_header(git_config_parser *reader, char **section_out)
 	do {
 		if (git__isspace(c)){
 			name[name_length] = '\0';
-			result = parse_section_header_ext(reader, line, name, section_out);
+			result = parse_section_header_ext(reader, line, pos, name, section_out);
 			git__free(line);
 			git__free(name);
 			return result;
diff --git a/tests/config/read.c b/tests/config/read.c
index ccc479bc1..008dfd9fc 100644
--- a/tests/config/read.c
+++ b/tests/config/read.c
@@ -779,6 +779,76 @@ void test_config_read__bom(void)
 	git_buf_dispose(&buf);
 }
 
+void test_config_read__arbitrary_whitespace_before_subsection(void)
+{
+	git_buf buf = GIT_BUF_INIT;
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "[some \t \"subsection\"]\n var = value\n");
+	cl_git_pass(git_config_open_ondisk(&cfg, "./testconfig"));
+	cl_git_pass(git_config_get_string_buf(&buf, cfg, "some.subsection.var"));
+	cl_assert_equal_s(buf.ptr, "value");
+
+	git_config_free(cfg);
+	git_buf_dispose(&buf);
+}
+
+void test_config_read__no_whitespace_after_subsection(void)
+{
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "[some \"subsection\" ]\n var = value\n");
+	cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	git_config_free(cfg);
+}
+
+void test_config_read__invalid_space_section(void)
+{
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "\xEF\xBB\xBF[some section]\n var = value\n");
+	cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	git_config_free(cfg);
+}
+
+void test_config_read__invalid_quoted_first_section(void)
+{
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "\xEF\xBB\xBF[\"some\"]\n var = value\n");
+	cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	git_config_free(cfg);
+}
+
+void test_config_read__invalid_unquoted_subsection(void)
+{
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "\xEF\xBB\xBF[some sub section]\n var = value\n");
+	cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	git_config_free(cfg);
+}
+
+void test_config_read__invalid_quoted_third_section(void)
+{
+	git_config *cfg;
+
+	cl_set_cleanup(&clean_test_config, NULL);
+	cl_git_mkfile("./testconfig", "\xEF\xBB\xBF[some sub \"section\"]\n var = value\n");
+	cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
+
+	git_config_free(cfg);
+}
+
 void test_config_read__single_line(void)
 {
 	git_buf buf = GIT_BUF_INIT;