From add8db06f91f1dc9acc7f20f8229f746041de2c8 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Mar 2014 15:28:29 -0700 Subject: [PATCH 1/3] Fix use-after-free in submodule reload If the first call to release a no-longer-existent submodule freed the object, the check if a second is needed would dereference the data that was just freed. --- src/submodule.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/submodule.c b/src/submodule.c index b07c9d917..0a3762fab 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -852,10 +852,13 @@ int git_submodule_reload_all(git_repository *repo, int force) git_strmap_foreach_value(repo->submodules, sm, { git_strmap *cache = repo->submodules; - if ((sm->flags & GIT_SUBMODULE_STATUS__IN_FLAGS) == 0) { - submodule_cache_remove_item(cache, sm->name, sm, true); + if (sm && (sm->flags & GIT_SUBMODULE_STATUS__IN_FLAGS) == 0) { + /* we must check path != name before first remove, in case + * that call frees the submodule */ + bool free_as_path = (sm->path != sm->name); - if (sm->path != sm->name) + submodule_cache_remove_item(cache, sm->name, sm, true); + if (free_as_path) submodule_cache_remove_item(cache, sm->path, sm, true); } }); From acdc7cff2e31223aa91d9421b9b4edc53ca87869 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Mar 2014 15:29:17 -0700 Subject: [PATCH 2/3] Fix memory leak of submodule branch name --- src/submodule.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/submodule.c b/src/submodule.c index 0a3762fab..e1500b847 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -1126,6 +1126,7 @@ static void submodule_release(git_submodule *sm) git__free(sm->path); git__free(sm->name); git__free(sm->url); + git__free(sm->branch); git__memzero(sm, sizeof(*sm)); git__free(sm); } From dae8ba6e0968d3ace5ac3c2878fb2072f1db43ba Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Mar 2014 15:29:32 -0700 Subject: [PATCH 3/3] Fix memory leak of test repository object --- tests/diff/submodules.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/diff/submodules.c b/tests/diff/submodules.c index 80dfcaa3f..ead5c71b6 100644 --- a/tests/diff/submodules.c +++ b/tests/diff/submodules.c @@ -449,7 +449,6 @@ void test_diff_submodules__skips_empty_includes_used(void) git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff *diff = NULL; diff_expects exp; - git_repository *r2; /* A side effect of of Git's handling of untracked directories and * auto-ignoring of ".git" entries is that a newly initialized Git @@ -469,7 +468,11 @@ void test_diff_submodules__skips_empty_includes_used(void) cl_assert_equal_i(0, exp.files); git_diff_free(diff); - cl_git_pass(git_repository_init(&r2, "empty_standard_repo/subrepo", 0)); + { + git_repository *r2; + cl_git_pass(git_repository_init(&r2, "empty_standard_repo/subrepo", 0)); + git_repository_free(r2); + } cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); memset(&exp, 0, sizeof(exp));