1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
|
From 8813bb5f4109991b88c98584a4abbb2d06cfbc28 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sat, 18 May 2024 10:32:45 +0000
Subject: hooks(clone protections): simplify templates hooks validation
commit eff37e9b1dec25a3e1297eb89a36d8e68fe01b40 upstream.
When an active hook is encountered during a clone operation, to protect
against Remote Code Execution attack vectors, Git checks whether the
hook was copied over from the templates directory.
When that logic was introduced, there was no other way to check this
than to add a function to compare files.
In the meantime, we've added code to compute the SHA-256 checksum of a
given hook and compare that checksum against a list of known-safe ones.
Let's simplify the logic by adding to said list when copying the
templates' hooks.
We need to be careful to support multi-process operations such as
recursive submodule clones: In such a scenario, the list of SHA-256
checksums that is kept in memory is not enough, we also have to pass the
information down to child processes via `GIT_CONFIG_PARAMETERS`.
Extend the regression test in t5601 to ensure that recursive clones are
handled as expected.
Note: Technically there is no way that the checksums computed while
initializing the submodules' gitdirs can be passed to the process that
performs the checkout: For historical reasons, these operations are
performed in processes spawned in separate loops from the
super-project's `git clone` process. But since the templates from which
the submodules are initialized are the very same as the ones from which
the super-project is initialized, we can get away with using the list of
SHA-256 checksums that is computed when initializing the super-project
and passing that down to the `submodule--helper` processes that perform
the recursive checkout.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
hook.c | 43 ++++++++++++++++---------------------------
hook.h | 10 ++++++++++
setup.c | 7 +++++++
t/t5601-clone.sh | 19 +++++++++++++++++++
4 files changed, 52 insertions(+), 27 deletions(-)
diff --git a/hook.c b/hook.c
index fc0548edb66..8ac51c9912b 100644
--- a/hook.c
+++ b/hook.c
@@ -14,32 +14,6 @@
#include "hash-ll.h"
#include "hex.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;
-}
-
static struct strset safe_hook_sha256s = STRSET_INIT;
static int safe_hook_sha256s_initialized;
@@ -70,6 +44,22 @@ static int get_sha256_of_file_contents(const char *path, char *sha256)
return 0;
}
+void add_safe_hook(const char *path)
+{
+ char sha256[GIT_SHA256_HEXSZ + 1] = { '\0' };
+
+ if (!get_sha256_of_file_contents(path, sha256)) {
+ char *p;
+
+ strset_add(&safe_hook_sha256s, sha256);
+
+ /* support multi-process operations e.g. recursive clones */
+ p = xstrfmt("safe.hook.sha256=%s", sha256);
+ git_config_push_parameter(p);
+ free(p);
+ }
+}
+
static int safe_hook_cb(const char *key, const char *value,
const struct config_context *ctx UNUSED, void *d)
{
@@ -142,7 +132,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) &&
!is_hook_safe_during_clone(name, path.buf, sha256))
die(_("active `%s` hook found during `git clone`:\n\t%s\n"
"For security reasons, this is disallowed by default.\n"
diff --git a/hook.h b/hook.h
index 19ab9a5806e..b4770d9bd88 100644
--- a/hook.h
+++ b/hook.h
@@ -87,4 +87,14 @@ int run_hooks(const char *hook_name);
* hook. This function behaves like the old run_hook_le() API.
*/
int run_hooks_l(const char *hook_name, ...);
+
+/**
+ * Mark the contents of the provided path as safe to run during a clone
+ * operation.
+ *
+ * This function is mainly used when copying templates to mark the
+ * just-copied hooks as benign.
+ */
+void add_safe_hook(const char *path);
+
#endif
diff --git a/setup.c b/setup.c
index 30f243fc32d..25828a85ec3 100644
--- a/setup.c
+++ b/setup.c
@@ -17,6 +17,8 @@
#include "trace2.h"
#include "worktree.h"
#include "exec-cmd.h"
+#include "run-command.h"
+#include "hook.h"
static int inside_git_dir = -1;
static int inside_work_tree = -1;
@@ -1868,6 +1870,7 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
size_t path_baselen = path->len;
size_t template_baselen = template_path->len;
struct dirent *de;
+ int is_hooks_dir = ends_with(template_path->buf, "/hooks/");
/* Note: if ".git/hooks" file exists in the repository being
* re-initialized, /etc/core-git/templates/hooks/update would
@@ -1920,6 +1923,10 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
strbuf_release(&lnk);
}
else if (S_ISREG(st_template.st_mode)) {
+ if (is_hooks_dir &&
+ is_executable(template_path->buf))
+ add_safe_hook(template_path->buf);
+
if (copy_file(path->buf, template_path->buf, st_template.st_mode))
die_errno(_("cannot copy '%s' to '%s'"),
template_path->buf, path->buf);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index deb1c282c71..ca3a8d1ebed 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -836,6 +836,25 @@ test_expect_success 'clone with init.templatedir runs hooks' '
git config --unset init.templateDir &&
test_grep ! "active .* hook found" err &&
test_path_is_missing hook-run-local-config/hook.run
+ ) &&
+
+ test_config_global protocol.file.allow always &&
+ git -C tmpl/hooks submodule add "$(pwd)/tmpl/hooks" sub &&
+ test_tick &&
+ git -C tmpl/hooks add .gitmodules sub &&
+ git -C tmpl/hooks commit -m submodule &&
+
+ (
+ sane_unset GIT_TEMPLATE_DIR &&
+ NO_SET_GIT_TEMPLATE_DIR=t &&
+ export NO_SET_GIT_TEMPLATE_DIR &&
+
+ git -c init.templateDir="$(pwd)/tmpl" \
+ clone --recurse-submodules \
+ tmpl/hooks hook-run-submodule 2>err &&
+ test_grep ! "active .* hook found" err &&
+ test_path_is_file hook-run-submodule/hook.run &&
+ test_path_is_file hook-run-submodule/sub/hook.run
)
'
|
