Commit 23c5699eb78724c4bdd8d2afcb7e49f32fbac6fb
2019-05-16T09:37:25
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129
diff --git a/src/config_parse.c b/src/config_parse.c
index 6b162cb..6e9b9a8 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 ccc479b..008dfd9 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;