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, 28 insertions, 9 deletions

diff --git a/src/submodule.c b/src/submodule.c
index a7f76d3c5..233957e43 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -220,7 +220,7 @@ static int load_submodule_names(git_strmap *out, git_repository *repo, git_confi
 	git_config_iterator *iter;
 	git_config_entry *entry;
 	git_buf buf = GIT_BUF_INIT;
-	int rval;
+	int rval, isvalid;
 	int error = 0;
 
 	if ((error = git_config_iterator_glob_new(&iter, cfg, key)) < 0)
@@ -232,7 +232,10 @@ static int load_submodule_names(git_strmap *out, git_repository *repo, git_confi
 		ldot = strrchr(entry->name, '.');
 
 		git_buf_put(&buf, fdot + 1, ldot - fdot - 1);
-		if (!git_submodule_name_is_valid(repo, buf.ptr, 0))
+		isvalid = git_submodule_name_is_valid(repo, buf.ptr, 0);
+		if (isvalid < 0)
+			return isvalid;
+		if (!isvalid)
 			continue;
 
 		git_strmap_insert(out, entry->value, git_buf_detach(&buf), &rval);
@@ -364,11 +367,25 @@ int git_submodule_lookup(
 
 int git_submodule_name_is_valid(const git_repository *repo, const char *name, int flags)
 {
+	git_buf buf = GIT_BUF_INIT;
+	int error, isvalid;
+
 	if (flags == 0)
 		flags = GIT_PATH_REJECT_FILESYSTEM_DEFAULTS;
 
+	/* Avoid allocating a new string if we can avoid it */
+	if (index(name, '\\')) {
+		if ((error = git_path_normalize_slashes(&buf, name)) < 0)
+			return error;
+	} else {
+		git_buf_attach_notowned(&buf, name, strlen(name));
+	}
+
 	/* FIXME: Un-consting it to reduce the amount of diff */
-	return git_path_isvalid((git_repository *)repo, name, flags);
+	isvalid =  git_path_isvalid((git_repository *)repo, buf.ptr, flags);
+	git_buf_free(&buf);
+
+	return isvalid;
 }
 
 static void submodule_free_dup(void *sm)
@@ -1571,16 +1588,17 @@ static int submodule_update_head(git_submodule *submodule)
 
 int git_submodule_reload(git_submodule *sm, int force)
 {
-	int error = 0;
+	int error = 0, isvalid;
 	git_config *mods;
 
 	GIT_UNUSED(force);
 
 	assert(sm);
 
-	if (!git_submodule_name_is_valid(sm->repo, sm->name, 0)) {
+	isvalid = git_submodule_name_is_valid(sm->repo, sm->name, 0);
+	if (isvalid <= 0) {
 		/* This should come with a warning, but we've no API for that */
-		return 0;
+		return isvalid;
 	}
 
 	if (!git_repository_is_bare(sm->repo)) {
@@ -1924,7 +1942,7 @@ static int submodule_load_each(const git_config_entry *entry, void *payload)
 	git_strmap *map = data->map;
 	git_buf name = GIT_BUF_INIT;
 	git_submodule *sm;
-	int error;
+	int error, isvalid;
 
 	if (git__prefixcmp(entry->name, "submodule.") != 0)
 		return 0;
@@ -1940,8 +1958,9 @@ static int submodule_load_each(const git_config_entry *entry, void *payload)
 	if ((error = git_buf_set(&name, namestart, property - namestart -1)) < 0)
 		return error;
 
-	if (!git_path_isvalid(data->repo, name.ptr, GIT_PATH_REJECT_TRAVERSAL)) {
-		error = 0;
+	isvalid = git_submodule_name_is_valid(data->repo, name.ptr, 0);
+	if (isvalid <= 0) {
+		error = isvalid;
 		goto done;
 	}