Skip to content

Commit

Permalink
WIP git clone protections: special-case Git LFS
Browse files Browse the repository at this point in the history
TODO:
- add support for specifying these hashes in the global config

- replace the logic to compare hooks against the template by populating
  an in-core list of known-good hashes

  (This might not actually work if we're dealing with sub-processes,
  dunno, need to check.)

As defense-in-depth measures, v2.39.4 and friends leading up to v2.45.1
introduced code that detects when hooks have been installed during a
`git clone`, which is indicative of a common attack vector with critical
severity that allows Remote Code Execution.

There are legitimate use cases for such behavior, though, for example
when those hooks stem from Git's own templates, which system
administrators are at liberty to modify to enforce, say, commit message
conventions. The git clone protections specifically add exceptions to
allow for that.

Another legitimate use case that has been identified too late to be
handled in these security bug-fix versions is Git LFS.

The symptom is that any Git LFS-enabled `git clone` will fail with an
error message that suggests to set `GIT_CLONE_PROTECTION_ACTIVE=false`
and try again. While the clone still works in many circumstances, and
while the result can be fixed using `git lfs pull` instead of
re-cloning, it is still a concerning usability issue.

Let's quickly follow up with code to special-case Git LFS' hooks, too:
They have a quite fixed format that has not been changed since
git-lfs/git-lfs@86a8d7369c98#diff-2ef0c6acf2db4fdf031483aa31f30f52c12f5b660969f14e3de000eedbf25d1c
which was first shipped with Git LFS v3.4.0 in July 2023. That allows
for checking the SHA-256 checksums of those files' contents to verify
that they are part of a benign scenario rather than an attack.

Signed-off-by: Johannes Schindelin <[email protected]>
  • Loading branch information
dscho committed May 17, 2024
1 parent 2745200 commit 367f5e7
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 1 deletion.
40 changes: 39 additions & 1 deletion hook.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,43 @@ static int identical_to_template_hook(const char *name, const char *path)
return ret;
}

static int is_valid_git_lfs_hook(const char *name, const char *path)
{
struct strbuf sb = STRBUF_INIT;
int fd, res;
const char *expected;

if (!strcmp("pre-push", name))
expected = "df5417b2daa3aa144c19681d1e997df7ebfe144fb7e3e05138bd80ae998008e4";
else if (!strcmp("post-checkout", name))
expected = "791471b4ff472aab844a4fceaa48bbb0a12193616f971e8e940625498b4938a6";
else if (!strcmp("post-commit", name))
expected = "21e961572bb3f43a5f2fbafc1cc764d86046cc2e5f0bbecebfe9684a0b73b664";
else if (!strcmp("post-merge", name))
expected = "75da0da66a803b4b030ad50801ba57062c6196105eb1d2251590d100edb9390b";
else
return 0;

if ((fd = open(path, O_RDONLY)) < 0)
return 0;
res = strbuf_read(&sb, fd, 400);
close(fd);
if (!res) {
git_hash_ctx ctx;
const struct git_hash_algo *sha256 = hash_algos + GIT_HASH_SHA256;
unsigned char hash[GIT_MAX_RAWSZ];

sha256->init_fn(&ctx);
sha256->update_fn(&ctx, sb.buf, sb.len);
sha256->final_fn(hash, &ctx);

res = !strcmp(expected, hash_to_hex(hash));
}

strbuf_release(&sb);
return res > 0;
}

const char *find_hook(const char *name)
{
static struct strbuf path = STRBUF_INIT;
Expand Down Expand Up @@ -65,7 +102,8 @@ 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))
!identical_to_template_hook(name, path.buf) &&
!is_valid_git_lfs_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 "
Expand Down
21 changes: 21 additions & 0 deletions t/t1800-hook.sh
Original file line number Diff line number Diff line change
Expand Up @@ -177,4 +177,25 @@ test_expect_success 'git hook run a hook with a bad shebang' '
test_cmp expect actual
'

write_lfs_pre_push_hook () {
write_script "$1" <<-\EOF
#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { echo >&2 "\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'pre-push' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks').\n"; exit 2; }
git lfs pre-push "$@"
EOF
}

test_expect_success 'Git LFS special-handling in clone protections' '
git init lfs-hooks &&
write_lfs_pre_push_hook lfs-hooks/.git/hooks/pre-push &&
write_script git-lfs <<-\EOF &&
echo "called $*" >fake-git-lfs.log
EOF
PATH="$PWD:$PATH" GIT_CLONE_PROTECTION_ACTIVE=true \
git -C lfs-hooks hook run pre-push &&
test_write_lines "called pre-push" >expect &&
test_cmp lfs-hooks/fake-git-lfs.log expect
'

test_done

0 comments on commit 367f5e7

Please sign in to comment.