packages/git
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
git/CVE-2019-1387.patch

182 lines
5.9 KiB
Diff
Raw Blame History

From a8dee3ca610f5a1d403634492136c887f83b59d2 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] 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(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f3ba6aa..a083b99 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1416,6 +1416,10 @@ static int module_clone(int argc, const char **argv, const char *prefix)
} 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);
diff --git a/submodule.c b/submodule.c
index 0f199c5..9da7181 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1993,6 +1993,47 @@ int submodule_move_head(const char *path,
return ret;
}
+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))
+ 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.
@@ -2000,7 +2041,7 @@ int submodule_move_head(const char *path,
static void relocate_single_git_dir_into_superproject(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))
@@ -2018,10 +2059,14 @@ static void relocate_single_git_dir_into_superproject(const char *path)
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);
fprintf(stderr, _("Migrating git directory of '%s%s' from\n'%s' to\n'%s'\n"),
get_super_prefix_or_empty(), path,
diff --git a/submodule.h b/submodule.h
index 8072e6d..c81ec1a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -124,6 +124,11 @@ int push_unpushed_submodules(struct repository *r,
*/
int submodule_to_gitdir(struct strbuf *buf, const char *submodule);
+/*
+ * 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 SUBMODULE_MOVE_HEAD_DRY_RUN (1<<0)
#define SUBMODULE_MOVE_HEAD_FORCE (1<<1)
int submodule_move_head(const char *path,
diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh
index 3008ba8..a44578f 100755
--- a/t/t7415-submodule-names.sh
+++ b/t/t7415-submodule-names.sh
@@ -224,4 +224,27 @@ test_expect_success MINGW 'prevent git~1 squatting on Windows' '
! grep gitdir squatting-clone/d/a/git~2
'
+test_expect_success 'git dirs of sibling submodules must not be nested' '
+ git init nested &&
+ test_commit -C nested nested &&
+ (
+ cd nested &&
+ cat >.gitmodules <<-EOF &&
+ [submodule "hippo"]
+ url = .
+ path = thing1
+ [submodule "hippo/hooks"]
+ url = .
+ path = thing2
+ EOF
+ git clone . thing1 &&
+ git clone . thing2 &&
+ git add .gitmodules thing1 thing2 &&
+ test_tick &&
+ git commit -m nested
+ ) &&
+ test_must_fail git clone --recurse-submodules nested clone 2>err &&
+ test_i18ngrep "is inside git dir" err
+'
+
test_done
--
1.8.3.1
Reference in New Issue View Git Blame Copy Permalink