| From 77346ef60d83c01922ba2c7ffbff6b22800215f2 Mon Sep 17 00:00:00 2001 |
| From: Johannes Schindelin <johannes.schindelin@gmx.de> |
| Date: Tue, 1 Oct 2019 23:27:18 +0200 |
| Subject: 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> |
| (cherry picked from commit a8dee3ca610f5a1d403634492136c887f83b59d2) |
| Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> |
| --- |
| 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 e103abbcbc..b93d624a85 100644 |
| --- a/builtin/submodule--helper.c |
| +++ b/builtin/submodule--helper.c |
| @@ -1411,6 +1411,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 6415cc5580..37d29bb252 100644 |
| --- a/submodule.c |
| +++ b/submodule.c |
| @@ -1732,6 +1732,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. |
| @@ -1740,7 +1781,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix, |
| 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)) |
| @@ -1758,10 +1799,14 @@ static void relocate_single_git_dir_into_superproject(const char *prefix, |
| 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 a680214c01..ac206dc182 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 9b69aed48e..71391fe161 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 |
| -- |
| 2.24.0.393.g34dc348eaf |
| |