submodule: ignore submodules which include path traversal in their name

If the we decide that the "name" of the submodule (i.e. its path inside
`.git/modules/`) is trying to escape that directory or otherwise trick us, we
ignore the configuration for that submodule.

This leaves us with a half-configured submodule when looking it up by path, but
it's the same result as if the configuration really were missing.

The name check is potentially more strict than it needs to be, but it lets us
re-use the check we're doing for the checkout. The function that encapsulates
this logic is ready to be exported but we don't want to do that in a security
release so it remains internal for now.

2 files changed, 38 insertions, 3 deletions

diff --git a/src/submodule.c b/src/submodule.c
index b7b28acd5..a7f76d3c5 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -214,7 +214,7 @@ static void free_submodule_names(git_strmap *names)
  * TODO: for some use-cases, this might need case-folding on a
  * case-insensitive filesystem
  */
-static int load_submodule_names(git_strmap *out, git_config *cfg)
+static int load_submodule_names(git_strmap *out, git_repository *repo, git_config *cfg)
 {
 	const char *key = "submodule\\..*\\.path";
 	git_config_iterator *iter;
@@ -232,6 +232,9 @@ static int load_submodule_names(git_strmap *out, git_config *cfg)
 		ldot = strrchr(entry->name, '.');
 
 		git_buf_put(&buf, fdot + 1, ldot - fdot - 1);
+		if (!git_submodule_name_is_valid(repo, buf.ptr, 0))
+			continue;
+
 		git_strmap_insert(out, entry->value, git_buf_detach(&buf), &rval);
 		if (rval < 0) {
 			giterr_set(GITERR_NOMEMORY, "error inserting submodule into hash table");
@@ -359,6 +362,15 @@ int git_submodule_lookup(
 	return 0;
 }
 
+int git_submodule_name_is_valid(const git_repository *repo, const char *name, int flags)
+{
+	if (flags == 0)
+		flags = GIT_PATH_REJECT_FILESYSTEM_DEFAULTS;
+
+	/* FIXME: Un-consting it to reduce the amount of diff */
+	return git_path_isvalid((git_repository *)repo, name, flags);
+}
+
 static void submodule_free_dup(void *sm)
 {
 	git_submodule_free(sm);
@@ -404,7 +416,7 @@ static int submodules_from_index(git_strmap *map, git_index *idx, git_config *cf
 	git_strmap *names = 0;
 
 	git_strmap_alloc(&names);
-	if ((error = load_submodule_names(names, cfg)))
+	if ((error = load_submodule_names(names, git_index_owner(idx), cfg)))
 		goto done;
 
 	if ((error = git_iterator_for_index(&i, git_index_owner(idx), idx, NULL)) < 0)
@@ -456,7 +468,7 @@ static int submodules_from_head(git_strmap *map, git_tree *head, git_config *cfg
 	const git_index_entry *entry;
 	git_strmap *names = 0;
 	git_strmap_alloc(&names);
-	if ((error = load_submodule_names(names, cfg)))
+	if ((error = load_submodule_names(names, git_tree_owner(head), cfg)))
 		goto done;
 
 	if ((error = git_iterator_for_tree(&i, head, NULL)) < 0)
@@ -1566,6 +1578,11 @@ int git_submodule_reload(git_submodule *sm, int force)
 
 	assert(sm);
 
+	if (!git_submodule_name_is_valid(sm->repo, sm->name, 0)) {
+		/* This should come with a warning, but we've no API for that */
+		return 0;
+	}
+
 	if (!git_repository_is_bare(sm->repo)) {
 		/* refresh config data */
 		if ((error = gitmodules_snapshot(&mods, sm->repo)) < 0 && error != GIT_ENOTFOUND)
@@ -1923,6 +1940,11 @@ 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;
+		goto done;
+	}
+
 	/*
 	 * Now that we have the submodule's name, we can use that to
 	 * figure out whether it's in the map. If it's not, we create
diff --git a/src/submodule.h b/src/submodule.h
index 72867a322..e188dbb3c 100644
--- a/src/submodule.h
+++ b/src/submodule.h
@@ -148,4 +148,17 @@ extern int git_submodule_parse_update(
 extern int git_submodule__map(
 	git_repository *repo,
 	git_strmap *map);
+
+/**
+ * Check whether a submodule's name is valid.
+ *
+ * Check the path against the path validity rules, either the filesystem
+ * defaults (like checkout does) or whichever you want to compare against.
+ *
+ * @param repo the repository which contains the submodule
+ * @param name the name to check
+ * @param flags the `GIT_PATH` flags to use for the check (0 to use filesystem defaults)
+ */
+extern int git_submodule_name_is_valid(const git_repository *repo, const char *name, int flags);
+
 #endif