Merge pull request libgit2#2212 from libgit2/rb/submodule-use-after-free

Fix use-after-free in submodule reload code and other memory leaks

Author: Vicent Marti

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);
 		}
 	});
@@ -1123,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);
 }

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));