From 1f4a91f7c2b938b6b9a2c7bd86db9c9d9d90fc7f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 22 Mar 2024 11:19:22 +0100 Subject: [PATCH] submodules: submodule paths must not contain symlinks When creating a submodule path, we must be careful not to follow symbolic links. Otherwise we may follow a symbolic link pointing to a gitdir (which are valid symbolic links!) e.g. while cloning. On case-insensitive filesystems, however, we blindly replace a directory that has been created as part of the `clone` operation with a symlink when the path to the latter differs only in case from the former's path. Let's simply avoid this situation by expecting not ever having to overwrite any existing file/directory/symlink upon cloning. That way, we won't even replace a directory that we just created. This addresses CVE-2024-32002. Reported-by: Filip Hejsek Signed-off-by: Johannes Schindelin Reference: https://git.kernel.org/pub/scm/git/git.git/commit/?id=97065761333fd62db1912d81b489db938d8c991d Conflicts: builtin/submodule--helper.c t/t7406-submodule-update.sh Signed-off-by: qiaojijun --- builtin/submodule--helper.c | 36 ++++++++++++++++++++++++++++ t/t7406-submodule-update.sh | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 46c03d2..e83d188 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1377,6 +1377,28 @@ static void prepare_possible_alternates(const char *sm_name, free(error_strategy); } +static int dir_contains_only_dotgit(const char *path) +{ + DIR *dir = opendir(path); + struct dirent *e; + int ret = 1; + + if (!dir) + return 0; + + e = readdir_skip_dot_and_dotdot(dir); + if (!e) + ret = 0; + else if (strcmp(DEFAULT_GIT_DIR_ENVIRONMENT, e->d_name) || + (e = readdir_skip_dot_and_dotdot(dir))) { + error("unexpected item '%s' in '%s'", e->d_name, path); + ret = 0; + } + + closedir(dir); + return ret; +} + static int module_clone(int argc, const char **argv, const char *prefix) { const char *name = NULL, *url = NULL, *depth = NULL; @@ -1388,6 +1410,8 @@ static int module_clone(int argc, const char **argv, const char *prefix) int dissociate = 0, require_init = 0; char *sm_alternate = NULL, *error_strategy = NULL; int single_branch = -1; + struct stat st; + struct option module_clone_options[] = { OPT_STRING(0, "prefix", &prefix, @@ -1450,6 +1474,10 @@ static int module_clone(int argc, const char **argv, const char *prefix) "git dir"), sm_gitdir); if (!file_exists(sm_gitdir)) { + if (require_init && !stat(path, &st) && + !is_empty_dir(path)) + die(_("directory not empty: '%s'"), path); + if (safe_create_leading_directories_const(sm_gitdir) < 0) die(_("could not create directory '%s'"), sm_gitdir); @@ -1459,6 +1487,14 @@ static int module_clone(int argc, const char **argv, const char *prefix) quiet, progress, single_branch)) die(_("clone of '%s' into submodule path '%s' failed"), url, path); + + if (require_init && !stat(path, &st) && + !dir_contains_only_dotgit(path)) { + char *dot_git = xstrfmt("%s/.git", path); + unlink(dot_git); + free(dot_git); + die(_("directory not empty: '%s'"), path); + } } else { if (require_init && !access(path, X_OK) && !is_empty_dir(path)) die(_("directory not empty: '%s'"), path); diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 4fb447a..03283dc 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -1006,4 +1006,52 @@ test_expect_success 'git clone passes the parallel jobs config on to submodules' rm -rf super4 ' +test_expect_success CASE_INSENSITIVE_FS,SYMLINKS \ + 'submodule paths must not follow symlinks' ' + + # This is only needed because we want to run this in a self-contained + # test without having to spin up an HTTP server; However, it would not + # be needed in a real-world scenario where the submodule is simply + # hosted on a public site. + test_config_global protocol.file.allow always && + + # Make sure that Git tries to use symlinks on Windows + test_config_global core.symlinks true && + + tell_tale_path="$PWD/tell.tale" && + git init hook && + ( + cd hook && + mkdir -p y/hooks && + write_script y/hooks/post-checkout <<-EOF && + echo HOOK-RUN >&2 + echo hook-run >"$tell_tale_path" + EOF + git add y/hooks/post-checkout && + test_tick && + git commit -m post-checkout + ) && + + hook_repo_path="$(pwd)/hook" && + git init captain && + ( + cd captain && + git submodule add --name x/y "$hook_repo_path" A/modules/x && + test_tick && + git commit -m add-submodule && + + printf .git >dotgit.txt && + git hash-object -w --stdin dot-git.hash && + printf "120000 %s 0\ta\n" "$(cat dot-git.hash)" >index.info && + git update-index --index-info err && + grep "directory not empty" err && + test_path_is_missing "$tell_tale_path" +' + test_done -- 2.20.1