Skip to content

Commit 23c5699

Browse files
committed
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 parent b83bd03 commit 23c5699

File tree

2 files changed

+79
-10
lines changed

2 files changed

+79
-10
lines changed

src/config_parse.c

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,26 +66,25 @@ static int strip_comments(char *line, int in_quotes)
6666
}
6767

6868

69-
static int parse_section_header_ext(git_config_parser *reader, const char *line, const char *base_name, char **section_name)
69+
static int parse_section_header_ext(git_config_parser *reader, const char *line, size_t pos, const char *base_name, char **section_name)
7070
{
7171
int c, rpos;
72-
char *first_quote, *last_quote;
72+
const char *first_quote, *last_quote;
7373
const char *line_start = line;
7474
git_buf buf = GIT_BUF_INIT;
7575
size_t quoted_len, alloc_len, base_name_len = strlen(base_name);
7676

77-
/*
78-
* base_name is what came before the space. We should be at the
79-
* first quotation mark, except for now, line isn't being kept in
80-
* sync so we only really use it to calculate the length.
81-
*/
77+
/* Skip any additional whitespace before our section name */
78+
while (git__isspace(line[pos]))
79+
pos++;
8280

83-
first_quote = strchr(line, '"');
84-
if (first_quote == NULL) {
81+
/* We should be at the first quotation mark. */
82+
if (line[pos] != '"') {
8583
set_parse_error(reader, 0, "missing quotation marks in section header");
8684
goto end_error;
8785
}
8886

87+
first_quote = &line[pos];
8988
last_quote = strrchr(line, '"');
9089
quoted_len = last_quote - first_quote;
9190

@@ -192,7 +191,7 @@ static int parse_section_header(git_config_parser *reader, char **section_out)
192191
do {
193192
if (git__isspace(c)){
194193
name[name_length] = '\0';
195-
result = parse_section_header_ext(reader, line, name, section_out);
194+
result = parse_section_header_ext(reader, line, pos, name, section_out);
196195
git__free(line);
197196
git__free(name);
198197
return result;

tests/config/read.c

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,76 @@ void test_config_read__bom(void)
779779
git_buf_dispose(&buf);
780780
}
781781

782+
void test_config_read__arbitrary_whitespace_before_subsection(void)
783+
{
784+
git_buf buf = GIT_BUF_INIT;
785+
git_config *cfg;
786+
787+
cl_set_cleanup(&clean_test_config, NULL);
788+
cl_git_mkfile("./testconfig", "[some \t \"subsection\"]\n var = value\n");
789+
cl_git_pass(git_config_open_ondisk(&cfg, "./testconfig"));
790+
cl_git_pass(git_config_get_string_buf(&buf, cfg, "some.subsection.var"));
791+
cl_assert_equal_s(buf.ptr, "value");
792+
793+
git_config_free(cfg);
794+
git_buf_dispose(&buf);
795+
}
796+
797+
void test_config_read__no_whitespace_after_subsection(void)
798+
{
799+
git_config *cfg;
800+
801+
cl_set_cleanup(&clean_test_config, NULL);
802+
cl_git_mkfile("./testconfig", "[some \"subsection\" ]\n var = value\n");
803+
cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
804+
805+
git_config_free(cfg);
806+
}
807+
808+
void test_config_read__invalid_space_section(void)
809+
{
810+
git_config *cfg;
811+
812+
cl_set_cleanup(&clean_test_config, NULL);
813+
cl_git_mkfile("./testconfig", "\xEF\xBB\xBF[some section]\n var = value\n");
814+
cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
815+
816+
git_config_free(cfg);
817+
}
818+
819+
void test_config_read__invalid_quoted_first_section(void)
820+
{
821+
git_config *cfg;
822+
823+
cl_set_cleanup(&clean_test_config, NULL);
824+
cl_git_mkfile("./testconfig", "\xEF\xBB\xBF[\"some\"]\n var = value\n");
825+
cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
826+
827+
git_config_free(cfg);
828+
}
829+
830+
void test_config_read__invalid_unquoted_subsection(void)
831+
{
832+
git_config *cfg;
833+
834+
cl_set_cleanup(&clean_test_config, NULL);
835+
cl_git_mkfile("./testconfig", "\xEF\xBB\xBF[some sub section]\n var = value\n");
836+
cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
837+
838+
git_config_free(cfg);
839+
}
840+
841+
void test_config_read__invalid_quoted_third_section(void)
842+
{
843+
git_config *cfg;
844+
845+
cl_set_cleanup(&clean_test_config, NULL);
846+
cl_git_mkfile("./testconfig", "\xEF\xBB\xBF[some sub \"section\"]\n var = value\n");
847+
cl_git_fail(git_config_open_ondisk(&cfg, "./testconfig"));
848+
849+
git_config_free(cfg);
850+
}
851+
782852
void test_config_read__single_line(void)
783853
{
784854
git_buf buf = GIT_BUF_INIT;

0 commit comments

Comments
 (0)