submodule: also validate Windows-separated paths for validity

Otherwise we would also admit `..\..\foo\bar` as a valid path and fail to
protect Windows users.

Ideally we would check for both separators without the need for the copied
string, but this'll get us over the RCE.

1 files changed, 37 insertions, 6 deletions

diff --git a/tests/submodule/escape.c b/tests/submodule/escape.c
index 576d7884b..0a3c4a3b8 100644
--- a/tests/submodule/escape.c
+++ b/tests/submodule/escape.c
@@ -13,6 +13,8 @@ void test_submodule_escape__cleanup(void)
 }
 
 #define EVIL_SM_NAME "../../modules/evil"
+#define EVIL_SM_NAME_WINDOWS "..\\\\..\\\\modules\\\\evil"
+#define EVIL_SM_NAME_WINDOWS_UNESC "..\\..\\modules\\evil"
 
 static int find_evil(git_submodule *sm, const char *name, void *payload)
 {
@@ -20,7 +22,8 @@ static int find_evil(git_submodule *sm, const char *name, void *payload)
 
 	GIT_UNUSED(sm);
 
-	if (!git__strcmp(EVIL_SM_NAME, name))
+	if (!git__strcmp(EVIL_SM_NAME, name) ||
+	    !git__strcmp(EVIL_SM_NAME_WINDOWS_UNESC, name))
 		*foundit = true;
 
 	return 0;
@@ -29,7 +32,6 @@ static int find_evil(git_submodule *sm, const char *name, void *payload)
 void test_submodule_escape__from_gitdir(void)
 {
 	int foundit;
-	git_config *cfg;
 	git_submodule *sm;
 	git_buf buf = GIT_BUF_INIT;
 	unsigned int sm_location;
@@ -42,10 +44,6 @@ void test_submodule_escape__from_gitdir(void)
 			   "    path = testrepo\n"
 			   "    url = ../testrepo.git\n");
 
-	/* We also need to update the value in the config */
-	cl_git_pass(git_repository_config__weakptr(&cfg, g_repo));
-	cfg = NULL;
-
 	/* Find it all the different ways we know about it */
 	foundit = 0;
 	cl_git_pass(git_submodule_foreach(g_repo, find_evil, &foundit));
@@ -63,3 +61,36 @@ void test_submodule_escape__from_gitdir(void)
 	cl_assert_equal_i(GIT_SUBMODULE_STATUS_IN_INDEX | GIT_SUBMODULE_STATUS_IN_HEAD, sm_location);
 	git_submodule_free(sm);
 }
+
+void test_submodule_escape__from_gitdir_windows(void)
+{
+	int foundit;
+	git_submodule *sm;
+	git_buf buf = GIT_BUF_INIT;
+	unsigned int sm_location;
+
+	g_repo = setup_fixture_submodule_simple();
+
+	cl_git_pass(git_buf_joinpath(&buf, git_repository_workdir(g_repo), ".gitmodules"));
+	cl_git_rewritefile(buf.ptr,
+			   "[submodule \"" EVIL_SM_NAME_WINDOWS "\"]\n"
+			   "    path = testrepo\n"
+			   "    url = ../testrepo.git\n");
+
+	/* Find it all the different ways we know about it */
+	foundit = 0;
+	cl_git_pass(git_submodule_foreach(g_repo, find_evil, &foundit));
+	cl_assert_equal_i(0, foundit);
+	cl_git_fail_with(GIT_ENOTFOUND, git_submodule_lookup(&sm, g_repo, EVIL_SM_NAME_WINDOWS_UNESC));
+	/*
+	 * We do know about this as it's in the index and HEAD, but the data is
+	 * incomplete as there is no configured data for it (we pretend it
+	 * doesn't exist). This leaves us with an odd situation but it's
+	 * consistent with what we would do if we did add a submodule with no
+	 * configuration.
+	 */
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "testrepo"));
+	cl_git_pass(git_submodule_location(&sm_location, sm));
+	cl_assert_equal_i(GIT_SUBMODULE_STATUS_IN_INDEX | GIT_SUBMODULE_STATUS_IN_HEAD, sm_location);
+	git_submodule_free(sm);
+}