Skip to content

Commit

Permalink
clone: drop the protections where hooks aren't run
Browse files Browse the repository at this point in the history 
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 8db1e87 (clone: prevent hooks from running
during a clone, 2024-03-28).

Signed-off-by: Johannes Schindelin <[email protected]>
  • Loading branch information
@dscho
dscho committed May 20, 2024
1 parent cd14042 commit 0044a35
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 96 deletions.
12 changes: 1 addition & 11 deletions builtin/clone.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand Down
34 changes: 0 additions & 34 deletions hook.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
51 changes: 0 additions & 51 deletions t/t5601-clone.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 0044a35

Please sign in to comment.