Sync with 2.40.2

* maint-2.40: (39 commits)
  Git 2.40.2
  Git 2.39.4
  fsck: warn about symlink pointing inside a gitdir
  core.hooksPath: add some protection while cloning
  init.templateDir: consider this config setting protected
  clone: prevent hooks from running during a clone
  Add a helper function to compare file contents
  init: refactor the template directory discovery into its own function
  find_hook(): refactor the `STRIP_EXTENSION` logic
  clone: when symbolic links collide with directories, keep the latter
  entry: report more colliding paths
  t5510: verify that D/F confusion cannot lead to an RCE
  submodule: require the submodule path to contain directories only
  clone_submodule: avoid using `access()` on directories
  submodules: submodule paths must not contain symlinks
  clone: prevent clashing git dirs when cloning submodule in parallel
  t7423: add tests for symlinked submodule directories
  has_dir_name(): do not get confused by characters < '/'
  docs: document security issues around untrusted .git dirs
  upload-pack: disable lazy-fetching by default
  ...

4 files changed, 135 insertions, 26 deletions

diff --git a/builtin/clone.c b/builtin/clone.c
index 15f9912b4c..d6545d036f 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -330,7 +330,20 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 	int src_len, dest_len;
 	struct dir_iterator *iter;
 	int iter_status;
-	struct strbuf realpath = STRBUF_INIT;
+
+	/*
+	 * Refuse copying directories by default which aren't owned by us. The
+	 * code that performs either the copying or hardlinking is not prepared
+	 * to handle various edge cases where an adversary may for example
+	 * racily swap out files for symlinks. This can cause us to
+	 * inadvertently use the wrong source file.
+	 *
+	 * Furthermore, even if we were prepared to handle such races safely,
+	 * creating hardlinks across user boundaries is an inherently unsafe
+	 * operation as the hardlinked files can be rewritten at will by the
+	 * potentially-untrusted user. We thus refuse to do so by default.
+	 */
+	die_upon_dubious_ownership(NULL, NULL, src_repo);
 
 	mkdir_if_missing(dest->buf, 0777);
 
@@ -378,9 +391,27 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 		if (unlink(dest->buf) && errno != ENOENT)
 			die_errno(_("failed to unlink '%s'"), dest->buf);
 		if (!option_no_hardlinks) {
-			strbuf_realpath(&realpath, src->buf, 1);
-			if (!link(realpath.buf, dest->buf))
+			if (!link(src->buf, dest->buf)) {
+				struct stat st;
+
+				/*
+				 * Sanity-check whether the created hardlink
+				 * actually links to the expected file now. This
+				 * catches time-of-check-time-of-use bugs in
+				 * case the source file was meanwhile swapped.
+				 */
+				if (lstat(dest->buf, &st))
+					die(_("hardlink cannot be checked at '%s'"), dest->buf);
+				if (st.st_mode != iter->st.st_mode ||
+				    st.st_ino != iter->st.st_ino ||
+				    st.st_dev != iter->st.st_dev ||
+				    st.st_size != iter->st.st_size ||
+				    st.st_uid != iter->st.st_uid ||
+				    st.st_gid != iter->st.st_gid)
+					die(_("hardlink different from source at '%s'"), dest->buf);
+
 				continue;
+			}
 			if (option_local > 0)
 				die_errno(_("failed to create link '%s'"), dest->buf);
 			option_no_hardlinks = 1;
@@ -393,8 +424,6 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 		strbuf_setlen(src, src_len);
 		die(_("failed to iterate over '%s'"), src->buf);
 	}
-
-	strbuf_release(&realpath);
 }
 
 static void clone_local(const char *src_repo, const char *dest_repo)
@@ -930,6 +959,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	int submodule_progress;
 	int filter_submodules = 0;
 	int hash_algo;
+	const char *template_dir;
+	char *template_dir_dup = NULL;
 
 	struct transport_ls_refs_options transport_ls_refs_options =
 		TRANSPORT_LS_REFS_OPTIONS_INIT;
@@ -949,6 +980,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		usage_msg_opt(_("You must specify a repository to clone."),
 			builtin_clone_usage, builtin_clone_options);
 
+	xsetenv("GIT_CLONE_PROTECTION_ACTIVE", "true", 0 /* allow user override */);
+	template_dir = get_template_dir(option_template);
+	if (*template_dir && !is_absolute_path(template_dir))
+		template_dir = template_dir_dup =
+			absolute_pathdup(template_dir);
+	xsetenv("GIT_CLONE_TEMPLATE_DIR", template_dir, 1);
+
 	if (option_depth || option_since || option_not.nr)
 		deepen = 1;
 	if (option_single_branch == -1)
@@ -1096,7 +1134,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
+	init_db(git_dir, real_git_dir, template_dir, GIT_HASH_UNKNOWN, NULL,
 		INIT_DB_QUIET);
 
 	if (real_git_dir) {
@@ -1440,6 +1478,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	free(dir);
 	free(path);
 	free(repo_to_free);
+	free(template_dir_dup);
 	junk_mode = JUNK_LEAVE_ALL;
 
 	transport_ls_refs_options_release(&transport_ls_refs_options);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index aef4036105..848f7ec70f 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -19,10 +19,6 @@
 #include "worktree.h"
 #include "wrapper.h"
 
-#ifndef DEFAULT_GIT_TEMPLATE_DIR
-#define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
-#endif
-
 #ifdef NO_TRUSTABLE_FILEMODE
 #define TEST_FILEMODE 0
 #else
@@ -101,8 +97,9 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
 	}
 }
 
-static void copy_templates(const char *template_dir, const char *init_template_dir)
+static void copy_templates(const char *option_template)
 {
+	const char *template_dir = get_template_dir(option_template);
 	struct strbuf path = STRBUF_INIT;
 	struct strbuf template_path = STRBUF_INIT;
 	size_t template_len;
@@ -111,16 +108,8 @@ static void copy_templates(const char *template_dir, const char *init_template_d
 	DIR *dir;
 	char *to_free = NULL;
 
-	if (!template_dir)
-		template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
-	if (!template_dir)
-		template_dir = init_template_dir;
-	if (!template_dir)
-		template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);
-	if (!template_dir[0]) {
-		free(to_free);
+	if (!template_dir || !*template_dir)
 		return;
-	}
 
 	strbuf_addstr(&template_path, template_dir);
 	strbuf_complete(&template_path, '/');
@@ -208,7 +197,6 @@ static int create_default_files(const char *template_path,
 	int reinit;
 	int filemode;
 	struct strbuf err = STRBUF_INIT;
-	const char *init_template_dir = NULL;
 	const char *work_tree = get_git_work_tree();
 
 	/*
@@ -220,9 +208,7 @@ static int create_default_files(const char *template_path,
 	 * values (since we've just potentially changed what's available on
 	 * disk).
 	 */
-	git_config_get_pathname("init.templatedir", &init_template_dir);
-	copy_templates(template_path, init_template_dir);
-	free((char *)init_template_dir);
+	copy_templates(template_path);
 	git_config_clear();
 	reset_shared_repository();
 	git_config(git_default_config, NULL);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6bf8d666ce..7f6981e3be 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -302,6 +302,9 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item,
 	struct child_process cp = CHILD_PROCESS_INIT;
 	char *displaypath;
 
+	if (validate_submodule_path(path) < 0)
+		exit(128);
+
 	displaypath = get_submodule_displaypath(path, info->prefix,
 						info->super_prefix);
 
@@ -634,6 +637,9 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 		.free_removed_argv_elements = 1,
 	};
 
+	if (validate_submodule_path(path) < 0)
+		exit(128);
+
 	if (!submodule_from_path(the_repository, null_oid(), path))
 		die(_("no submodule mapping found in .gitmodules for path '%s'"),
 		      path);
@@ -1238,6 +1244,9 @@ static void sync_submodule(const char *path, const char *prefix,
 	if (!is_submodule_active(the_repository, path))
 		return;
 
+	if (validate_submodule_path(path) < 0)
+		exit(128);
+
 	sub = submodule_from_path(the_repository, null_oid(), path);
 
 	if (sub && sub->url) {
@@ -1381,6 +1390,9 @@ static void deinit_submodule(const char *path, const char *prefix,
 	struct strbuf sb_config = STRBUF_INIT;
 	char *sub_git_dir = xstrfmt("%s/.git", path);
 
+	if (validate_submodule_path(path) < 0)
+		exit(128);
+
 	sub = submodule_from_path(the_repository, null_oid(), path);
 
 	if (!sub || !sub->name)
@@ -1662,16 +1674,42 @@ static char *clone_submodule_sm_gitdir(const char *name)
 	return sm_gitdir;
 }
 
+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 clone_submodule(const struct module_clone_data *clone_data,
 			   struct string_list *reference)
 {
 	char *p;
 	char *sm_gitdir = clone_submodule_sm_gitdir(clone_data->name);
 	char *sm_alternate = NULL, *error_strategy = NULL;
+	struct stat st;
 	struct child_process cp = CHILD_PROCESS_INIT;
 	const char *clone_data_path = clone_data->path;
 	char *to_free = NULL;
 
+	if (validate_submodule_path(clone_data_path) < 0)
+		exit(128);
+
 	if (!is_absolute_path(clone_data->path))
 		clone_data_path = to_free = xstrfmt("%s/%s", get_git_work_tree(),
 						    clone_data->path);
@@ -1681,6 +1719,10 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 		      "git dir"), sm_gitdir);
 
 	if (!file_exists(sm_gitdir)) {
+		if (clone_data->require_init && !stat(clone_data_path, &st) &&
+		    !is_empty_dir(clone_data_path))
+			die(_("directory not empty: '%s'"), clone_data_path);
+
 		if (safe_create_leading_directories_const(sm_gitdir) < 0)
 			die(_("could not create directory '%s'"), sm_gitdir);
 
@@ -1725,10 +1767,18 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 		if(run_command(&cp))
 			die(_("clone of '%s' into submodule path '%s' failed"),
 			    clone_data->url, clone_data_path);
+
+		if (clone_data->require_init && !stat(clone_data_path, &st) &&
+		    !dir_contains_only_dotgit(clone_data_path)) {
+			char *dot_git = xstrfmt("%s/.git", clone_data_path);
+			unlink(dot_git);
+			free(dot_git);
+			die(_("directory not empty: '%s'"), clone_data_path);
+		}
 	} else {
 		char *path;
 
-		if (clone_data->require_init && !access(clone_data_path, X_OK) &&
+		if (clone_data->require_init && !stat(clone_data_path, &st) &&
 		    !is_empty_dir(clone_data_path))
 			die(_("directory not empty: '%s'"), clone_data_path);
 		if (safe_create_leading_directories_const(clone_data_path) < 0)
@@ -1738,6 +1788,23 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 		free(path);
 	}
 
+	/*
+	 * We already performed this check at the beginning of this function,
+	 * before cloning the objects. This tries to detect racy behavior e.g.
+	 * in parallel clones, where another process could easily have made the
+	 * gitdir nested _after_ it was created.
+	 *
+	 * To prevent further harm coming from this unintentionally-nested
+	 * gitdir, let's disable it by deleting the `HEAD` file.
+	 */
+	if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0) {
+		char *head = xstrfmt("%s/HEAD", sm_gitdir);
+		unlink(head);
+		free(head);
+		die(_("refusing to create/use '%s' in another submodule's "
+		      "git dir"), sm_gitdir);
+	}
+
 	connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
 
 	p = git_pathdup_submodule(clone_data_path, "config");
@@ -2513,6 +2580,9 @@ static int update_submodule(struct update_data *update_data)
 {
 	int ret;
 
+	if (validate_submodule_path(update_data->sm_path) < 0)
+		return -1;
+
 	ret = determine_submodule_update_strategy(the_repository,
 						  update_data->just_cloned,
 						  update_data->sm_path,
@@ -2620,12 +2690,21 @@ static int update_submodules(struct update_data *update_data)
 
 	for (i = 0; i < suc.update_clone_nr; i++) {
 		struct update_clone_data ucd = suc.update_clone[i];
-		int code;
+		int code = 128;
 
 		oidcpy(&update_data->oid, &ucd.oid);
 		update_data->just_cloned = ucd.just_cloned;
 		update_data->sm_path = ucd.sub->path;
 
+		/*
+		 * Verify that the submodule path does not contain any
+		 * symlinks; if it does, it might have been tampered with.
+		 * TODO: allow exempting it via
+		 * `safe.submodule.path` or something
+		 */
+		if (validate_submodule_path(update_data->sm_path) < 0)
+			goto fail;
+
 		code = ensure_core_worktree(update_data->sm_path);
 		if (code)
 			goto fail;
@@ -3336,6 +3415,9 @@ static int module_add(int argc, const char **argv, const char *prefix)
 	normalize_path_copy(add_data.sm_path, add_data.sm_path);
 	strip_dir_trailing_slashes(add_data.sm_path);
 
+	if (validate_submodule_path(add_data.sm_path) < 0)
+		exit(128);
+
 	die_on_index_match(add_data.sm_path, force);
 	die_on_repo_without_commits(add_data.sm_path);
 
diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c
index beb9dd0861..edb01aab70 100644
--- a/builtin/upload-pack.c
+++ b/builtin/upload-pack.c
@@ -37,6 +37,8 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
 
 	packet_trace_identity("upload-pack");
 	read_replace_refs = 0;
+	/* TODO: This should use NO_LAZY_FETCH_ENVIRONMENT */
+	xsetenv("GIT_NO_LAZY_FETCH", "1", 0);
 
 	argc = parse_options(argc, argv, prefix, options, upload_pack_usage, 0);