git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 5/6] clone: drop the protections where hooks aren't run

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
May 20, 2024, 20:22 UTC
Message-ID
<0044a35567417a552cc518576670b43f7141a02e.1716236526.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1732.v3.git.1716236526.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>

As part of the security bug-fix releases v2.39.4, ..., v2.45.1, I introduced logic to safeguard `git clone` from running hooks that were installed _during_ the clone operation.

The rationale was that Git's CVE-2024-32002, CVE-2021-21300, CVE-2019-1354, CVE-2019-1353, CVE-2019-1352, and CVE-2019-1349 should have been low-severity vulnerabilities but were elevated to critical/high severity by the attack vector that allows a weakness where files inside `.git/` can be inadvertently written during a `git clone` to escalate to a Remote Code Execution attack by virtue of installing a malicious `post-checkout` hook that Git will then run at the end of the operation without giving the user a chance to see what code is executed.

Unfortunately, Git LFS uses a similar strategy to install its own `post-checkout` hook during a `git clone`; In fact, Git LFS is installing four separate hooks while running the `smudge` filter.

While this pattern is probably in want of being improved by introducing better support in Git for Git LFS and other tools wishing to register hooks to be run at various stages of Git's commands, let's undo the clone protections to unbreak Git LFS-enabled clones.

This reverts commit 8db1e8743c0 (clone: prevent hooks from running during a clone, 2024-03-28).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/clone.c  | 12 +-----------
 hook.c           | 34 --------------------------------
 t/t5601-clone.sh | 51 ------------------------------------------------
 3 files changed, 1 insertion(+), 96 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index e7721f5c22c..9ec500d427e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -937,8 +937,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	int err = 0, complete_refs_before_fetch = 1;
 	int submodule_progress;
 	int filter_submodules = 0;
-	const char *template_dir;
-	char *template_dir_dup = NULL;
 
 	struct transport_ls_refs_options transport_ls_refs_options =
 		TRANSPORT_LS_REFS_OPTIONS_INIT;
@@ -958,13 +956,6 @@ 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)
@@ -1112,7 +1103,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	init_db(git_dir, real_git_dir, template_dir, GIT_HASH_UNKNOWN, NULL,
+	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
 		INIT_DB_QUIET);
 
 	if (real_git_dir) {
@@ -1430,7 +1421,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	free(unborn_head);
 	free(dir);
 	free(path);
-	free(template_dir_dup);
 	UNLEAK(repo);
 	junk_mode = JUNK_LEAVE_ALL;
 
diff --git a/hook.c b/hook.c
index fc974cee1d8..22b274b60b1 100644
--- a/hook.c
+++ b/hook.c
@@ -3,32 +3,6 @@
 #include "run-command.h"
 #include "config.h"
 
-static int identical_to_template_hook(const char *name, const char *path)
-{
-	const char *env = getenv("GIT_CLONE_TEMPLATE_DIR");
-	const char *template_dir = get_template_dir(env && *env ? env : NULL);
-	struct strbuf template_path = STRBUF_INIT;
-	int found_template_hook, ret;
-
-	strbuf_addf(&template_path, "%s/hooks/%s", template_dir, name);
-	found_template_hook = access(template_path.buf, X_OK) >= 0;
-#ifdef STRIP_EXTENSION
-	if (!found_template_hook) {
-		strbuf_addstr(&template_path, STRIP_EXTENSION);
-		found_template_hook = access(template_path.buf, X_OK) >= 0;
-	}
-#endif
-	if (!found_template_hook) {
-		strbuf_release(&template_path);
-		return 0;
-	}
-
-	ret = do_files_match(template_path.buf, path);
-
-	strbuf_release(&template_path);
-	return ret;
-}
-
 const char *find_hook(const char *name)
 {
 	static struct strbuf path = STRBUF_INIT;
@@ -64,14 +38,6 @@ const char *find_hook(const char *name)
 		}
 		return NULL;
 	}
-	if (!git_hooks_path && git_env_bool("GIT_CLONE_PROTECTION_ACTIVE", 0) &&
-	    !identical_to_template_hook(name, path.buf))
-		die(_("active `%s` hook found during `git clone`:\n\t%s\n"
-		      "For security reasons, this is disallowed by default.\n"
-		      "If this is intentional and the hook should actually "
-		      "be run, please\nrun the command again with "
-		      "`GIT_CLONE_PROTECTION_ACTIVE=false`"),
-		    name, path.buf);
 	return path.buf;
 }
 
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 20deca0231b..fd029843307 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -771,57 +771,6 @@ test_expect_success 'batch missing blob request does not inadvertently try to fe
 	git clone --filter=blob:limit=0 "file://$(pwd)/server" client
 '
 
-test_expect_success 'clone with init.templatedir runs hooks' '
-	git init tmpl/hooks &&
-	write_script tmpl/hooks/post-checkout <<-EOF &&
-	echo HOOK-RUN >&2
-	echo I was here >hook.run
-	EOF
-	git -C tmpl/hooks add . &&
-	test_tick &&
-	git -C tmpl/hooks commit -m post-checkout &&
-
-	test_when_finished "git config --global --unset init.templateDir || :" &&
-	test_when_finished "git config --unset init.templateDir || :" &&
-	(
-		sane_unset GIT_TEMPLATE_DIR &&
-		NO_SET_GIT_TEMPLATE_DIR=t &&
-		export NO_SET_GIT_TEMPLATE_DIR &&
-
-		git -c core.hooksPath="$(pwd)/tmpl/hooks" \
-			clone tmpl/hooks hook-run-hookspath 2>err &&
-		! grep "active .* hook found" err &&
-		test_path_is_file hook-run-hookspath/hook.run &&
-
-		git -c init.templateDir="$(pwd)/tmpl" \
-			clone tmpl/hooks hook-run-config 2>err &&
-		! grep "active .* hook found" err &&
-		test_path_is_file hook-run-config/hook.run &&
-
-		git clone --template=tmpl tmpl/hooks hook-run-option 2>err &&
-		! grep "active .* hook found" err &&
-		test_path_is_file hook-run-option/hook.run &&
-
-		git config --global init.templateDir "$(pwd)/tmpl" &&
-		git clone tmpl/hooks hook-run-global-config 2>err &&
-		git config --global --unset init.templateDir &&
-		! grep "active .* hook found" err &&
-		test_path_is_file hook-run-global-config/hook.run &&
-
-		# clone ignores local `init.templateDir`; need to create
-		# a new repository because we deleted `.git/` in the
-		# `setup` test case above
-		git init local-clone &&
-		cd local-clone &&
-
-		git config init.templateDir "$(pwd)/../tmpl" &&
-		git clone ../tmpl/hooks hook-run-local-config 2>err &&
-		git config --unset init.templateDir &&
-		! grep "active .* hook found" err &&
-		test_path_is_missing hook-run-local-config/hook.run
-	)
-'
-
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-- 
gitgitgadget
Previous: Johannes Schindelin via GitGitGadgetNext: Johannes Schindelin via GitGitGadget
Message 47 of 54 in “Various fixes for v2.45.1 and friends”
  1. 0/8 Various fixes for v2.45.1 and friendsJohannes Schindelin via GitGitGadget, May 17, 2024
  2. 1/8 hook: plug a new memory leakJohannes Schindelin via GitGitGadget, May 17, 2024
  3. 2/8 init: use the correct path of the templates directory againJohannes Schindelin via GitGitGadget, May 17, 2024
  4. 3/8 Revert "core.hooksPath: add some protection while cloning"Johannes Schindelin via GitGitGadget, May 17, 2024
  5. 4/8 tests: verify that `clone -c core.hooksPath=/dev/null` works againJohannes Schindelin via GitGitGadget, May 17, 2024
  6. Junio C HamanoMay 18, 2024
  7. Johannes SchindelinMay 18, 2024
  8. 5/8 hook(clone protections): add escape hatchJohannes Schindelin via GitGitGadget, May 17, 2024
  9. Junio C HamanoMay 18, 2024
  10. 6/8 hooks(clone protections): special-case current Git LFS hooksJohannes Schindelin via GitGitGadget, May 17, 2024
  11. Junio C HamanoMay 18, 2024
  12. 7/8 hooks(clone protections): simplify templates hooks validationJohannes Schindelin via GitGitGadget, May 17, 2024
  13. 8/8 Revert "Add a helper function to compare file contents"Johannes Schindelin via GitGitGadget, May 17, 2024
  14. Junio C HamanoMay 17, 2024
  15. Johannes SchindelinMay 18, 2024
  16. 0/8 Various fixes for v2.45.1 and friendsJohannes Schindelin via GitGitGadget, May 18, 2024
  17. 1/8 hook: plug a new memory leakJohannes Schindelin via GitGitGadget, May 18, 2024
  18. 2/8 init: use the correct path of the templates directory againJohannes Schindelin via GitGitGadget, May 18, 2024
  19. 3/8 Revert "core.hooksPath: add some protection while cloning"Johannes Schindelin via GitGitGadget, May 18, 2024
  20. 4/8 tests: verify that `clone -c core.hooksPath=/dev/null` works againJohannes Schindelin via GitGitGadget, May 18, 2024
  21. 5/8 hook(clone protections): add escape hatchJohannes Schindelin via GitGitGadget, May 18, 2024
  22. Jeff KingMay 18, 2024
  23. Junio C HamanoMay 18, 2024
  24. Jeff KingMay 18, 2024
  25. Johannes SchindelinMay 18, 2024
  26. Johannes SchindelinMay 18, 2024
  27. Jeff KingMay 18, 2024
  28. Johannes SchindelinMay 18, 2024
  29. Jeff KingMay 18, 2024
  30. Junio C HamanoMay 19, 2024
  31. Johannes SchindelinMay 20, 2024
  32. Junio C HamanoMay 20, 2024
  33. Johannes SchindelinMay 20, 2024
  34. Junio C HamanoMay 20, 2024
  35. Johannes SchindelinMay 20, 2024
  36. 6/8 hooks(clone protections): special-case current Git LFS hooksJohannes Schindelin via GitGitGadget, May 18, 2024
  37. 7/8 hooks(clone protections): simplify templates hooks validationJohannes Schindelin via GitGitGadget, May 18, 2024
  38. 8/8 Revert "Add a helper function to compare file contents"Johannes Schindelin via GitGitGadget, May 18, 2024
  39. Junio C HamanoMay 18, 2024
  40. Johannes SchindelinMay 18, 2024
  41. Johannes SchindelinMay 18, 2024
  42. 0/6 Various fixes for v2.45.1 and friendsJohannes Schindelin via GitGitGadget, May 20, 2024
  43. 1/6 hook: plug a new memory leakJohannes Schindelin via GitGitGadget, May 20, 2024
  44. 2/6 init: use the correct path of the templates directory againJohannes Schindelin via GitGitGadget, May 20, 2024
  45. 3/6 Revert "core.hooksPath: add some protection while cloning"Johannes Schindelin via GitGitGadget, May 20, 2024
  46. 4/6 tests: verify that `clone -c core.hooksPath=/dev/null` works againJohannes Schindelin via GitGitGadget, May 20, 2024
  47. 5/6 clone: drop the protections where hooks aren't runJohannes Schindelin via GitGitGadget, May 20, 2024
  48. 6/6 Revert "Add a helper function to compare file contents"Johannes Schindelin via GitGitGadget, May 20, 2024
  49. Junio C HamanoMay 20, 2024
  50. Junio C HamanoMay 21, 2024
  51. Junio C HamanoMay 21, 2024
  52. brian m. carlsonMay 21, 2024
  53. Junio C HamanoMay 21, 2024
  54. Junio C HamanoMay 21, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.