From 20f3588efc6cbcae5bbaabf65ee12df87b51a9ea Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 30 Mar 2024 19:12:50 +0100 Subject: [PATCH] core.hooksPath: add some protection while cloning Quite frequently, when vulnerabilities were found in Git's (quite complex) clone machinery, a relatively common way to escalate the severity was to trick Git into running a hook which is actually a script that has just been laid on disk as part of that clone. This constitutes a Remote Code Execution vulnerability, the highest severity observed in Git's vulnerabilities so far. Some previously-fixed vulnerabilities allowed malicious repositories to be crafted such that Git would check out files not in the worktree, but in, say, a submodule's `/hooks/` directory. A vulnerability that "merely" allows to modify the Git config would allow a related attack vector, to manipulate Git into looking in the worktree for hooks, e.g. redirecting the location where Git looks for hooks, via setting `core.hooksPath` (which would be classified as CWE-427: Uncontrolled Search Path Element and CWE-114: Process Control, for more details see https://cwe.mitre.org/data/definitions/427.html and https://cwe.mitre.org/data/definitions/114.html). To prevent that attack vector, let's error out and complain loudly if an active `core.hooksPath` configuration is seen in the repository-local Git config during a `git clone`. There is one caveat: This changes Git's behavior in a slightly backwards-incompatible manner. While it is probably a rare scenario (if it exists at all) to configure `core.hooksPath` via a config in the Git templates, it _is_ conceivable that some valid setup requires this to work. In the hopefully very unlikely case that a user runs into this, there is an escape hatch: set the `GIT_CLONE_PROTECTION_ACTIVE=false` environment variable. Obviously, this should be done only with utmost caution. Signed-off-by: Johannes Schindelin --- config.c | 13 ++++++++++++- t/t1800-hook.sh | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 8c1c4071f0..85b37f2ee0 100644 --- a/config.c +++ b/config.c @@ -1525,8 +1525,19 @@ static int git_default_core_config(const char *var, const char *value, void *cb) if (!strcmp(var, "core.attributesfile")) return git_config_pathname(&git_attributes_file, var, value); - if (!strcmp(var, "core.hookspath")) + if (!strcmp(var, "core.hookspath")) { + if (current_config_scope() == CONFIG_SCOPE_LOCAL && + git_env_bool("GIT_CLONE_PROTECTION_ACTIVE", 0)) + die(_("active `core.hooksPath` found in the local " + "repository config:\n\t%s\nFor security " + "reasons, this is disallowed by default.\nIf " + "this is intentional and the hook should " + "actually be run, please\nrun the command " + "again with " + "`GIT_CLONE_PROTECTION_ACTIVE=false`"), + value); return git_config_pathname(&git_hooks_path, var, value); + } if (!strcmp(var, "core.bare")) { is_bare_repository_cfg = git_config_bool(var, value); diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 2ef3579fa7..7ee12e6f48 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -177,4 +177,19 @@ test_expect_success 'git hook run a hook with a bad shebang' ' test_cmp expect actual ' +test_expect_success 'clone protections' ' + test_config core.hooksPath "$(pwd)/my-hooks" && + mkdir -p my-hooks && + write_script my-hooks/test-hook <<-\EOF && + echo Hook ran $1 + EOF + + git hook run test-hook 2>err && + grep "Hook ran" err && + test_must_fail env GIT_CLONE_PROTECTION_ACTIVE=true \ + git hook run test-hook 2>err && + grep "active .core.hooksPath" err && + ! grep "Hook ran" err +' + test_done -- 2.11.4.GIT