From bf662f7cf8daff2357923446cf9d22f5d4b4a66b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 5 Oct 2018 10:55:29 +0200 Subject: tests: always unlink created config files While our tests in config::include create a plethora of configuration files, most of them do not get removed at the end of each test. This can cause weird interactions with tests that are being run at a later stage if these later tests try to create files or directories with the same name as any of the created configuration files. Fix the issue by unlinking all created files at the end of these tests. --- tests/config/include.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/config/include.c b/tests/config/include.c index d2ea2c2fd..ff8ac251c 100644 --- a/tests/config/include.c +++ b/tests/config/include.c @@ -35,6 +35,8 @@ void test_config_include__absolute(void) cl_git_pass(git_config_get_string_buf(&buf, cfg, "foo.bar.baz")); cl_assert_equal_s("huzzah", git_buf_cstr(&buf)); + + cl_git_pass(p_unlink("config-include-absolute")); } void test_config_include__homedir(void) @@ -48,6 +50,8 @@ void test_config_include__homedir(void) cl_assert_equal_s("huzzah", git_buf_cstr(&buf)); cl_sandbox_set_search_path_defaults(); + + cl_git_pass(p_unlink("config-include-homedir")); } /* We need to pretend that the variables were defined where the file was included */ @@ -66,6 +70,9 @@ void test_config_include__ordering(void) git_buf_clear(&buf); cl_git_pass(git_config_get_string_buf(&buf, cfg, "foo.bar.baz")); cl_assert_equal_s("huzzah", git_buf_cstr(&buf)); + + cl_git_pass(p_unlink("included")); + cl_git_pass(p_unlink("including")); } /* We need to pretend that the variables were defined where the file was included */ @@ -76,8 +83,8 @@ void test_config_include__depth(void) cl_git_fail(git_config_open_ondisk(&cfg, "a")); - p_unlink("a"); - p_unlink("b"); + cl_git_pass(p_unlink("a")); + cl_git_pass(p_unlink("b")); } void test_config_include__missing(void) @@ -89,6 +96,8 @@ void test_config_include__missing(void) cl_assert(giterr_last() == NULL); cl_git_pass(git_config_get_string_buf(&buf, cfg, "foo.bar")); cl_assert_equal_s("baz", git_buf_cstr(&buf)); + + cl_git_pass(p_unlink("including")); } void test_config_include__missing_homedir(void) @@ -103,6 +112,7 @@ void test_config_include__missing_homedir(void) cl_assert_equal_s("baz", git_buf_cstr(&buf)); cl_sandbox_set_search_path_defaults(); + cl_git_pass(p_unlink("including")); } #define replicate10(s) s s s s s s s s s s @@ -122,6 +132,10 @@ void test_config_include__depth2(void) git_buf_clear(&buf); cl_git_pass(git_config_get_string_buf(&buf, cfg, "foo.bar2")); cl_assert_equal_s("baz2", git_buf_cstr(&buf)); + + cl_git_pass(p_unlink("top-level")); + cl_git_pass(p_unlink("middle")); + cl_git_pass(p_unlink("bottom")); } void test_config_include__removing_include_removes_values(void) @@ -132,6 +146,9 @@ void test_config_include__removing_include_removes_values(void) cl_git_pass(git_config_open_ondisk(&cfg, "top-level")); cl_git_mkfile("top-level", ""); cl_git_fail(git_config_get_string_buf(&buf, cfg, "foo.bar")); + + cl_git_pass(p_unlink("top-level")); + cl_git_pass(p_unlink("included")); } void test_config_include__rewriting_include_refreshes_values(void) @@ -145,6 +162,10 @@ void test_config_include__rewriting_include_refreshes_values(void) cl_git_fail(git_config_get_string_buf(&buf, cfg, "foo.bar")); cl_git_pass(git_config_get_string_buf(&buf, cfg, "first.other")); cl_assert_equal_s(buf.ptr, "value"); + + cl_git_pass(p_unlink("top-level")); + cl_git_pass(p_unlink("first")); + cl_git_pass(p_unlink("second")); } void test_config_include__included_variables_cannot_be_deleted(void) @@ -154,13 +175,20 @@ void test_config_include__included_variables_cannot_be_deleted(void) cl_git_pass(git_config_open_ondisk(&cfg, "top-level")); cl_git_fail(git_config_delete_entry(cfg, "foo.bar")); + + cl_git_pass(p_unlink("top-level")); + cl_git_pass(p_unlink("included")); } void test_config_include__included_variables_cannot_be_modified(void) { cl_git_mkfile("top-level", "[include]\npath = included\n"); + cl_git_mkfile("included", "[foo]\nbar = value"); cl_git_pass(git_config_open_ondisk(&cfg, "top-level")); cl_git_fail(git_config_set_string(cfg, "foo.bar", "other-value")); + + cl_git_pass(p_unlink("top-level")); + cl_git_pass(p_unlink("included")); } -- cgit v1.2.1 From d06d4220eec035466d1a837972a40546b8904330 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 5 Oct 2018 10:56:02 +0200 Subject: config_file: properly ignore includes without "path" value In case a configuration includes a key "include.path=" without any value, the generated configuration entry will have its value set to `NULL`. This is unexpected by the logic handling includes, and as soon as we try to calculate the included path we will unconditionally dereference that `NULL` pointer and thus segfault. Fix the issue by returning early in both `parse_include` and `parse_conditional_include` in case where the `file` argument is `NULL`. Add a test to avoid future regression. The issue has been found by the oss-fuzz project, issue 10810. --- src/config_file.c | 5 ++++- tests/config/include.c | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/config_file.c b/src/config_file.c index e8740d35f..57db97d8b 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -664,6 +664,9 @@ static int parse_include(git_config_parser *reader, char *dir; int result; + if (!file) + return 0; + if ((result = git_path_dirname_r(&path, reader->file->path)) < 0) return result; @@ -765,7 +768,7 @@ static int parse_conditional_include(git_config_parser *reader, size_t i; int error = 0, matches; - if (!parse_data->repo) + if (!parse_data->repo || !file) return 0; condition = git__substrdup(section + strlen("includeIf."), diff --git a/tests/config/include.c b/tests/config/include.c index ff8ac251c..bab59bcbe 100644 --- a/tests/config/include.c +++ b/tests/config/include.c @@ -87,6 +87,16 @@ void test_config_include__depth(void) cl_git_pass(p_unlink("b")); } +void test_config_include__empty_path_sanely_handled(void) +{ + cl_git_mkfile("a", "[include]\npath"); + cl_git_pass(git_config_open_ondisk(&cfg, "a")); + cl_git_pass(git_config_get_string_buf(&buf, cfg, "include.path")); + cl_assert_equal_s("", git_buf_cstr(&buf)); + + cl_git_pass(p_unlink("a")); +} + void test_config_include__missing(void) { cl_git_mkfile("including", "[include]\npath = nonexistentfile\n[foo]\nbar = baz"); -- cgit v1.2.1