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 `<git>/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 <johannes.schindelin@gmx.de>

diff --git a/config.c b/config.c
index 8c1c4071f0d1a7012a7f1d67da46736c2ac982bf..85b37f2ee09d0a8a1491fa95c66e26411899568d 100644 (file)
--- 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 2ef3579fa7c23db5055c41e68438d39871bcf6e9..7ee12e6f48afab2b811c6848333f86090c2a5f1e 100755 (executable)
--- 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