From 3bf82059cb0650463230305c2db95f41507a0f1b Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Tue, 1 Oct 2019 23:27:18 +0200
Subject: [PATCH 08/17] Disallow dubiously-nested submodule git directories

Currently it is technically possible to let a submodule's git
directory point right into the git dir of a sibling submodule.

Example: the git directories of two submodules with the names `hippo`
and `hippo/hooks` would be `.git/modules/hippo/` and
`.git/modules/hippo/hooks/`, respectively, but the latter is already
intended to house the former's hooks.

In most cases, this is just confusing, but there is also a (quite
contrived) attack vector where Git can be fooled into mistaking remote
content for file contents it wrote itself during a recursive clone.

Let's plug this bug.

To do so, we introduce the new function `validate_submodule_git_dir()`
which simply verifies that no git dir exists for any leading directories
of the submodule name (if there are any).

Note: this patch specifically continues to allow sibling modules names
of the form `core/lib`, `core/doc`, etc, as long as `core` is not a
submodule name.

This fixes CVE-2019-1387.

Reported-by: Nicolas Joly <Nicolas.Joly@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/submodule--helper.c |  4 +++
 submodule.c                 | 49 +++++++++++++++++++++++++++++++++++--
 submodule.h                 |  5 ++++
 t/t7415-submodule-names.sh  | 23 +++++++++++++++++
 4 files changed, 79 insertions(+), 2 deletions(-)

Index: git-2.12.3/builtin/submodule--helper.c
===================================================================
--- git-2.12.3.orig/builtin/submodule--helper.c
+++ git-2.12.3/builtin/submodule--helper.c
@@ -636,6 +636,10 @@ static int module_clone(int argc, const
 	} else
 		path = xstrdup(path);
 
+	if (validate_submodule_git_dir(sm_gitdir, name) < 0)
+		die(_("refusing to create/use '%s' in another submodule's "
+			"git dir"), sm_gitdir);
+
 	if (!file_exists(sm_gitdir)) {
 		if (safe_create_leading_directories_const(sm_gitdir) < 0)
 			die(_("could not create directory '%s'"), sm_gitdir);
Index: git-2.12.3/submodule.c
===================================================================
--- git-2.12.3.orig/submodule.c
+++ git-2.12.3/submodule.c
@@ -1383,6 +1383,47 @@ void prepare_submodule_repo_env(struct a
 			 DEFAULT_GIT_DIR_ENVIRONMENT);
 }
 
+int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
+{
+	size_t len = strlen(git_dir), suffix_len = strlen(submodule_name);
+	char *p;
+	int ret = 0;
+
+	if (len <= suffix_len || (p = git_dir + len - suffix_len)[-1] != '/' ||
+	    strcmp(p, submodule_name))
+		die("BUG: submodule name '%s' not a suffix of git dir '%s'",
+		    submodule_name, git_dir);
+
+	/*
+	 * We prevent the contents of sibling submodules' git directories to
+	 * clash.
+	 *
+	 * Example: having a submodule named `hippo` and another one named
+	 * `hippo/hooks` would result in the git directories
+	 * `.git/modules/hippo/` and `.git/modules/hippo/hooks/`, respectively,
+	 * but the latter directory is already designated to contain the hooks
+	 * of the former.
+	 */
+	for (; *p; p++) {
+		if (is_dir_sep(*p)) {
+			char c = *p;
+
+			*p = '\0';
+			if (is_git_directory(git_dir))
+				ret = -1;
+			*p = c;
+
+			if (ret < 0)
+				return error(_("submodule git dir '%s' is "
+					       "inside git dir '%.*s'"),
+					     git_dir,
+					     (int)(p - git_dir), git_dir);
+		}
+	}
+
+	return 0;
+}
+
 /*
  * Embeds a single submodules git directory into the superprojects git dir,
  * non recursively.
@@ -1391,7 +1432,7 @@ static void relocate_single_git_dir_into
 						      const char *path)
 {
 	char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
-	const char *new_git_dir;
+	char *new_git_dir;
 	const struct submodule *sub;
 
 	if (submodule_uses_worktrees(path))
@@ -1409,10 +1450,14 @@ static void relocate_single_git_dir_into
 	if (!sub)
 		die(_("could not lookup name for submodule '%s'"), path);
 
-	new_git_dir = git_path("modules/%s", sub->name);
+	new_git_dir = git_pathdup("modules/%s", sub->name);
+	if (validate_submodule_git_dir(new_git_dir, sub->name) < 0)
+		die(_("refusing to move '%s' into an existing git dir"),
+		    real_old_git_dir);
 	if (safe_create_leading_directories_const(new_git_dir) < 0)
 		die(_("could not create directory '%s'"), new_git_dir);
 	real_new_git_dir = real_pathdup(new_git_dir, 1);
+	free(new_git_dir);
 
 	if (!prefix)
 		prefix = get_super_prefix();
Index: git-2.12.3/submodule.h
===================================================================
--- git-2.12.3.orig/submodule.h
+++ git-2.12.3/submodule.h
@@ -89,6 +89,11 @@ extern int parallel_submodules(void);
  */
 extern void prepare_submodule_repo_env(struct argv_array *out);
 
+/*
+ * Make sure that no submodule's git dir is nested in a sibling submodule's.
+ */
+int validate_submodule_git_dir(char *git_dir, const char *submodule_name);
+
 #define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0)
 extern void absorb_git_dir_into_superproject(const char *prefix,
 					     const char *path,
