From a5f43b95364e76436df25349f5f7b07e532e8e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 31 May 2011 13:48:44 +0200 Subject: [PATCH 1/9] Config file open: don't free memory that doesn't belong to us MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On error, it would free the configuration object even though it didn't own that memory, which would cause a double-free. This fixes the first part of Issue #210 Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/config_file.c b/src/config_file.c index ace7cc8ff..216c07417 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -261,7 +261,6 @@ static int config_open(git_config_file *cfg) cleanup: cvar_list_free(&b->var_list); gitfo_free_buf(&b->reader.buffer); - free(cfg); return git__rethrow(error, "Failed to open config"); } From f2abee47d8908cd74f1ca2e46f30c8b9f6a07754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 29 May 2011 01:24:09 +0200 Subject: [PATCH 2/9] cfg_readline: really ignore empty lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify cfg_readline and at the same time fix it so that it does really ignore empty lines. This fixes point 2-1 of Issue #210 Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 39 ++++++++------------------------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index 216c07417..e468a0b88 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -483,15 +483,6 @@ static int cfg_peek(diskfile_backend *cfg, int flags) return ret; } -static const char *LINEBREAK_UNIX = "\\\n"; -static const char *LINEBREAK_WIN32 = "\\\r\n"; - -static int is_linebreak(const char *pos) -{ - return memcmp(pos - 1, LINEBREAK_UNIX, sizeof(LINEBREAK_UNIX)) == 0 || - memcmp(pos - 2, LINEBREAK_WIN32, sizeof(LINEBREAK_WIN32)) == 0; -} - /* * Read and consume a line, returning it in newly-allocated memory. */ @@ -502,38 +493,24 @@ static char *cfg_readline(diskfile_backend *cfg) int line_len; line_src = cfg->reader.read_ptr; + + /* Skip empty empty lines */ + while (isspace(*line_src)) + ++line_src; + line_end = strchr(line_src, '\n'); /* no newline at EOF */ if (line_end == NULL) line_end = strchr(line_src, 0); - else - while (is_linebreak(line_end)) - line_end = strchr(line_end + 1, '\n'); + line_len = line_end - line_src; - while (line_src < line_end && isspace(*line_src)) - line_src++; - - line = (char *)git__malloc((size_t)(line_end - line_src) + 1); + line = git__malloc(line_len + 1); if (line == NULL) return NULL; - line_len = 0; - while (line_src < line_end) { - - if (memcmp(line_src, LINEBREAK_UNIX, sizeof(LINEBREAK_UNIX)) == 0) { - line_src += sizeof(LINEBREAK_UNIX); - continue; - } - - if (memcmp(line_src, LINEBREAK_WIN32, sizeof(LINEBREAK_WIN32)) == 0) { - line_src += sizeof(LINEBREAK_WIN32); - continue; - } - - line[line_len++] = *line_src++; - } + memcpy(line, line_src, line_len); line[line_len] = '\0'; From 7bc9e2aa2fe4b554021129fabb8a46b5a33ed4e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 31 May 2011 15:06:22 +0200 Subject: [PATCH 3/9] Guard against double-freeing the current section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If parse_section_header{,_ext} return an error, current_section doesn't get allocated. Set it to NULL after freeing so we don't try to free it again. This fixes part 2-2 of Issue #210. Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index e468a0b88..37e3f1f15 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -808,6 +808,7 @@ static int config_parse(diskfile_backend *cfg_file) case '[': /* section header, new section begins */ free(current_section); + current_section = NULL; error = parse_section_header(cfg_file, ¤t_section); break; @@ -847,8 +848,7 @@ static int config_parse(diskfile_backend *cfg_file) } } - if (current_section) - free(current_section); + free(current_section); return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to parse config"); } From 7288d8b65c5f584f14eff045ab2f70a97a666898 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 31 May 2011 15:11:49 +0200 Subject: [PATCH 4/9] Parse section header ext: don't leak on error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also free the subsection if we find too many quotes Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index 37e3f1f15..4b7ce069e 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -597,8 +597,10 @@ static int parse_section_header_ext(const char *line, const char *base_name, cha do { switch (c) { case '"': - if (quote_marks++ >= 2) - return git__throw(GIT_EOBJCORRUPTED, "Failed to parse ext header. Too many quotes"); + if (quote_marks++ >= 2) { + error = git__throw(GIT_EOBJCORRUPTED, "Failed to parse ext header. Too many quotes"); + goto out; + } break; case '\\': c = line[rpos++]; From 5892277cd04bb9ed0dee5bea5fdf96f95aac0a8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 31 May 2011 15:16:25 +0200 Subject: [PATCH 5/9] Config parse header ext: don't allow text after closing quote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing is allowed betwen the closing quotation mark and the ] so return an error if there is. Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index 4b7ce069e..57611a762 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -595,12 +595,15 @@ static int parse_section_header_ext(const char *line, const char *base_name, cha * added to the string. In case of error, jump to out */ do { + if (quote_marks == 2) { + error = git__throw(GIT_EOBJCORRUPTED, "Falied to parse ext header. Text after closing quote"); + goto out; + + } + switch (c) { case '"': - if (quote_marks++ >= 2) { - error = git__throw(GIT_EOBJCORRUPTED, "Failed to parse ext header. Too many quotes"); - goto out; - } + ++quote_marks; break; case '\\': c = line[rpos++]; @@ -612,6 +615,7 @@ static int parse_section_header_ext(const char *line, const char *base_name, cha error = git__throw(GIT_EOBJCORRUPTED, "Failed to parse ext header. Unsupported escape char \\%c", c); goto out; } + break; default: break; } From 38d0bc1e818c5e776a39a334c83054ff96abb07c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 31 May 2011 16:06:01 +0200 Subject: [PATCH 6/9] Add config test for empty line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Martín Nieto --- tests/resources/config/config6 | Bin 0 -> 84 bytes tests/t15-config.c | 15 +++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 tests/resources/config/config6 diff --git a/tests/resources/config/config6 b/tests/resources/config/config6 new file mode 100644 index 0000000000000000000000000000000000000000..0f8f90ac9ad952d1aa16883afa11394d13a22ce3 GIT binary patch literal 84 zcmazpOU%hkQBW!_O)5@JF3HT#Q;OwM0D|KD+|-hc%)E33TZNLM(o`<4XoQebYEE$~ L7KLevKyfYrtxy}1 literal 0 HcmV?d00001 diff --git a/tests/t15-config.c b/tests/t15-config.c index c2e146cf1..a93caad75 100644 --- a/tests/t15-config.c +++ b/tests/t15-config.c @@ -159,6 +159,20 @@ BEGIN_TEST(config5, "test number suffixes") git_config_free(cfg); END_TEST +BEGIN_TEST(config6, "test blank lines") + git_config *cfg; + int i; + + must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config6")); + + must_pass(git_config_get_bool(cfg, "valid.subsection.something", &i)); + must_be_true(i == 1); + + must_pass(git_config_get_bool(cfg, "something.else.something", &i)); + must_be_true(i == 0); + + git_config_free(cfg); +END_TEST BEGIN_SUITE(config) ADD_TEST(config0); @@ -167,4 +181,5 @@ BEGIN_SUITE(config) ADD_TEST(config3); ADD_TEST(config4); ADD_TEST(config5); + ADD_TEST(config6); END_SUITE From 30d0550da8e73d409407fe01133628c9594d63fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 31 May 2011 16:31:19 +0200 Subject: [PATCH 7/9] Add test for invalid ext header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Martín Nieto --- tests/resources/config/config7 | Bin 0 -> 113 bytes tests/t15-config.c | 8 ++++++++ 2 files changed, 8 insertions(+) create mode 100644 tests/resources/config/config7 diff --git a/tests/resources/config/config7 b/tests/resources/config/config7 new file mode 100644 index 0000000000000000000000000000000000000000..6af6fcf25186986d8d2f6efcf8b7ec6f25180880 GIT binary patch literal 113 zcmXxcK?;B%5QX8rr+B#O5Yb7b#aL4clLKj4x6g>)_J4ftB#AR@K78h>L``-yXv?~B s2{El0K&D4!%$m9a6444u_g85tykip0@7EgFn4h!PXL;e!7%vpe185y2!~g&Q literal 0 HcmV?d00001 diff --git a/tests/t15-config.c b/tests/t15-config.c index a93caad75..4a3c0dc9f 100644 --- a/tests/t15-config.c +++ b/tests/t15-config.c @@ -174,6 +174,13 @@ BEGIN_TEST(config6, "test blank lines") git_config_free(cfg); END_TEST +BEGIN_TEST(config7, "test for invalid ext headers") + git_config *cfg; + + must_fail(git_config_open_file(&cfg, CONFIG_BASE "/config7")); + +END_TEST + BEGIN_SUITE(config) ADD_TEST(config0); ADD_TEST(config1); @@ -182,4 +189,5 @@ BEGIN_SUITE(config) ADD_TEST(config4); ADD_TEST(config5); ADD_TEST(config6); + ADD_TEST(config7); END_SUITE From c7e6e95841920055239b28b7dd146792895f3025 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 31 May 2011 17:44:55 +0200 Subject: [PATCH 8/9] Don't try to parse an empty config file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Martín Nieto --- src/config_file.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/config_file.c b/src/config_file.c index 57611a762..d76c6024d 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -802,6 +802,10 @@ static int config_parse(diskfile_backend *cfg_file) cfg_file->reader.read_ptr = cfg_file->reader.buffer.data; cfg_file->reader.eof = 0; + /* If the file is empty, there's nothing for us to do */ + if (*cfg_file->reader.read_ptr == '\0') + return GIT_SUCCESS; + skip_bom(cfg_file); while (error == GIT_SUCCESS && !cfg_file->reader.eof) { From fc0ee5bdd384f6f9a6cf1ca1e09d9578fe72fdbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 31 May 2011 17:54:50 +0200 Subject: [PATCH 9/9] Add test for empty config file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Martín Nieto --- tests/resources/config/config8 | Bin tests/t15-config.c | 9 +++++++++ 2 files changed, 9 insertions(+) create mode 100644 tests/resources/config/config8 diff --git a/tests/resources/config/config8 b/tests/resources/config/config8 new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/tests/t15-config.c b/tests/t15-config.c index 4a3c0dc9f..08a2cdbf2 100644 --- a/tests/t15-config.c +++ b/tests/t15-config.c @@ -181,6 +181,14 @@ BEGIN_TEST(config7, "test for invalid ext headers") END_TEST +BEGIN_TEST(config8, "don't fail on empty files") + git_config *cfg; + + must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config8")); + + git_config_free(cfg); +END_TEST + BEGIN_SUITE(config) ADD_TEST(config0); ADD_TEST(config1); @@ -190,4 +198,5 @@ BEGIN_SUITE(config) ADD_TEST(config5); ADD_TEST(config6); ADD_TEST(config7); + ADD_TEST(config8); END_SUITE