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
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
|
Origin: https://github.com/git/git/commit/3b0bf2704980b1ed6018622bdf5377ec22289688
Origin: https://github.com/git/git/commit/5f1a3fec8c304decaa9af2bf503712050a4a84e0
Origin: https://github.com/git/git/commit/ae9abbb63eea74441e3e8b153dc6ec1f94c373b4
Origin: https://github.com/git/git/commit/b9063afda17a2aa6310423c9f7b776c41f753091
Origin: https://github.com/git/git/commit/6b11e3d52e919cce91011f4f9025e6f4b61375f2
Reviewed-by: Aron Xu <aron@debian.org>
Last-Updated: 2023-01-26
diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt
index 6d764fe..74627c5 100644
--- a/Documentation/config/safe.txt
+++ b/Documentation/config/safe.txt
@@ -26,3 +26,17 @@ directory was listed in the `safe.directory` list. If `safe.directory=*`
is set in system config and you want to re-enable this protection, then
initialize your list with an empty value before listing the repositories
that you deem safe.
++
+As explained, Git only allows you to access repositories owned by
+yourself, i.e. the user who is running Git, by default. When Git
+is running as 'root' in a non Windows platform that provides sudo,
+however, git checks the SUDO_UID environment variable that sudo creates
+and will allow access to the uid recorded as its value in addition to
+the id from 'root'.
+This is to make it easy to perform a common sequence during installation
+"make && sudo make install". A git process running under 'sudo' runs as
+'root' but the 'sudo' command exports the environment variable to record
+which id the original user has.
+If that is not what you would prefer and want git to only trust
+repositories that are owned by root instead, then you can remove
+the `SUDO_UID` variable from root's environment before invoking git.
diff --git a/git-compat-util.h b/git-compat-util.h
index 63ba89d..f505f81 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -393,12 +393,68 @@ static inline int git_offset_1st_component(const char *path)
#endif
#ifndef is_path_owned_by_current_user
+
+#ifdef __TANDEM
+#define ROOT_UID 65535
+#else
+#define ROOT_UID 0
+#endif
+
+/*
+ * Do not use this function when
+ * (1) geteuid() did not say we are running as 'root', or
+ * (2) using this function will compromise the system.
+ *
+ * PORTABILITY WARNING:
+ * This code assumes uid_t is unsigned because that is what sudo does.
+ * If your uid_t type is signed and all your ids are positive then it
+ * should all work fine.
+ * If your version of sudo uses negative values for uid_t or it is
+ * buggy and return an overflowed value in SUDO_UID, then git might
+ * fail to grant access to your repository properly or even mistakenly
+ * grant access to someone else.
+ * In the unlikely scenario this happened to you, and that is how you
+ * got to this message, we would like to know about it; so sent us an
+ * email to git@vger.kernel.org indicating which platform you are
+ * using and which version of sudo, so we can improve this logic and
+ * maybe provide you with a patch that would prevent this issue again
+ * in the future.
+ */
+static inline void extract_id_from_env(const char *env, uid_t *id)
+{
+ const char *real_uid = getenv(env);
+
+ /* discard anything empty to avoid a more complex check below */
+ if (real_uid && *real_uid) {
+ char *endptr = NULL;
+ unsigned long env_id;
+
+ errno = 0;
+ /* silent overflow errors could trigger a bug here */
+ env_id = strtoul(real_uid, &endptr, 10);
+ if (!*endptr && !errno)
+ *id = env_id;
+ }
+}
+
static inline int is_path_owned_by_current_uid(const char *path)
{
struct stat st;
+ uid_t euid;
+
if (lstat(path, &st))
return 0;
- return st.st_uid == geteuid();
+
+ euid = geteuid();
+ if (euid == ROOT_UID)
+ {
+ if (st.st_uid == ROOT_UID)
+ return 1;
+ else
+ extract_id_from_env("SUDO_UID", &euid);
+ }
+
+ return st.st_uid == euid;
}
#define is_path_owned_by_current_user is_path_owned_by_current_uid
diff --git a/setup.c b/setup.c
index aad9ace..9dcecda 100644
--- a/setup.c
+++ b/setup.c
@@ -1054,14 +1054,32 @@ static int safe_directory_cb(const char *key, const char *value, void *d)
return 0;
}
-static int ensure_valid_ownership(const char *path)
+/*
+ * Check if a repository is safe, by verifying the ownership of the
+ * worktree (if any), the git directory, and the gitfile (if any).
+ *
+ * Exemptions for known-safe repositories can be added via `safe.directory`
+ * config settings; for non-bare repositories, their worktree needs to be
+ * added, for bare ones their git directory.
+ */
+static int ensure_valid_ownership(const char *gitfile,
+ const char *worktree, const char *gitdir)
{
- struct safe_directory_data data = { .path = path };
+ struct safe_directory_data data = {
+ .path = worktree ? worktree : gitdir
+ };
if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
- is_path_owned_by_current_user(path))
+ (!gitfile || is_path_owned_by_current_user(gitfile)) &&
+ (!worktree || is_path_owned_by_current_user(worktree)) &&
+ (!gitdir || is_path_owned_by_current_user(gitdir)))
return 1;
+ /*
+ * data.path is the "path" that identifies the repository and it is
+ * constant regardless of what failed above. data.is_safe should be
+ * initialized to false, and might be changed by the callback.
+ */
read_very_early_config(safe_directory_cb, &data);
return data.is_safe;
@@ -1149,6 +1167,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
current_device = get_device_or_die(dir->buf, NULL, 0);
for (;;) {
int offset = dir->len, error_code = 0;
+ char *gitdir_path = NULL;
+ char *gitfile = NULL;
if (offset > min_offset)
strbuf_addch(dir, '/');
@@ -1159,21 +1179,50 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
if (die_on_error ||
error_code == READ_GITFILE_ERR_NOT_A_FILE) {
/* NEEDSWORK: fail if .git is not file nor dir */
- if (is_git_directory(dir->buf))
+ if (is_git_directory(dir->buf)) {
gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ gitdir_path = xstrdup(dir->buf);
+ }
} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)
return GIT_DIR_INVALID_GITFILE;
- }
+ } else
+ gitfile = xstrdup(dir->buf);
+ /*
+ * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT
+ * to check that directory for a repository.
+ * Now trim that tentative addition away, because we want to
+ * focus on the real directory we are in.
+ */
strbuf_setlen(dir, offset);
if (gitdirenv) {
- if (!ensure_valid_ownership(dir->buf))
- return GIT_DIR_INVALID_OWNERSHIP;
- strbuf_addstr(gitdir, gitdirenv);
- return GIT_DIR_DISCOVERED;
+ enum discovery_result ret;
+
+ if (ensure_valid_ownership(gitfile,
+ dir->buf,
+ (gitdir_path ? gitdir_path : gitdirenv))) {
+ strbuf_addstr(gitdir, gitdirenv);
+ ret = GIT_DIR_DISCOVERED;
+ } else
+ ret = GIT_DIR_INVALID_OWNERSHIP;
+
+ /*
+ * Earlier, during discovery, we might have allocated
+ * string copies for gitdir_path or gitfile so make
+ * sure we don't leak by freeing them now, before
+ * leaving the loop and function.
+ *
+ * Note: gitdirenv will be non-NULL whenever these are
+ * allocated, therefore we need not take care of releasing
+ * them outside of this conditional block.
+ */
+ free(gitdir_path);
+ free(gitfile);
+
+ return ret;
}
if (is_git_directory(dir->buf)) {
- if (!ensure_valid_ownership(dir->buf))
+ if (!ensure_valid_ownership(NULL, NULL, dir->buf))
return GIT_DIR_INVALID_OWNERSHIP;
strbuf_addstr(gitdir, ".");
return GIT_DIR_BARE;
@@ -1306,7 +1355,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
struct strbuf quoted = STRBUF_INIT;
sq_quote_buf_pretty("ed, dir.buf);
- die(_("unsafe repository ('%s' is owned by someone else)\n"
+ die(_("detected dubious ownership in repository at '%s'\n"
"To add an exception for this directory, call:\n"
"\n"
"\tgit config --global --add safe.directory %s"),
diff --git a/t/lib-sudo.sh b/t/lib-sudo.sh
new file mode 100644
index 0000000..b4d7788
--- /dev/null
+++ b/t/lib-sudo.sh
@@ -0,0 +1,15 @@
+# Helpers for running git commands under sudo.
+
+# Runs a scriplet passed through stdin under sudo.
+run_with_sudo () {
+ local ret
+ local RUN="$TEST_DIRECTORY/$$.sh"
+ write_script "$RUN" "$TEST_SHELL_PATH"
+ # avoid calling "$RUN" directly so sudo doesn't get a chance to
+ # override the shell, add aditional restrictions or even reject
+ # running the script because its security policy deem it unsafe
+ sudo "$TEST_SHELL_PATH" -c "\"$RUN\""
+ ret=$?
+ rm -f "$RUN"
+ return $ret
+}
diff --git a/t/t0034-root-safe-directory.sh b/t/t0034-root-safe-directory.sh
new file mode 100755
index 0000000..ff31176
--- /dev/null
+++ b/t/t0034-root-safe-directory.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+
+test_description='verify safe.directory checks while running as root'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-sudo.sh
+
+if [ "$GIT_TEST_ALLOW_SUDO" != "YES" ]
+then
+ skip_all="You must set env var GIT_TEST_ALLOW_SUDO=YES in order to run this test"
+ test_done
+fi
+
+if ! test_have_prereq NOT_ROOT
+then
+ skip_all="These tests do not support running as root"
+ test_done
+fi
+
+test_lazy_prereq SUDO '
+ sudo -n id -u >u &&
+ id -u root >r &&
+ test_cmp u r &&
+ command -v git >u &&
+ sudo command -v git >r &&
+ test_cmp u r
+'
+
+if ! test_have_prereq SUDO
+then
+ skip_all="Your sudo/system configuration is either too strict or unsupported"
+ test_done
+fi
+
+test_expect_success SUDO 'setup' '
+ sudo rm -rf root &&
+ mkdir -p root/r &&
+ (
+ cd root/r &&
+ git init
+ )
+'
+
+test_expect_success SUDO 'sudo git status as original owner' '
+ (
+ cd root/r &&
+ git status &&
+ sudo git status
+ )
+'
+
+test_expect_success SUDO 'setup root owned repository' '
+ sudo mkdir -p root/p &&
+ sudo git init root/p
+'
+
+test_expect_success 'cannot access if owned by root' '
+ (
+ cd root/p &&
+ test_must_fail git status
+ )
+'
+
+test_expect_success 'can access if addressed explicitly' '
+ (
+ cd root/p &&
+ GIT_DIR=.git GIT_WORK_TREE=. git status
+ )
+'
+
+test_expect_success SUDO 'can access with sudo if root' '
+ (
+ cd root/p &&
+ sudo git status
+ )
+'
+
+test_expect_success SUDO 'can access with sudo if root by removing SUDO_UID' '
+ (
+ cd root/p &&
+ run_with_sudo <<-END
+ unset SUDO_UID &&
+ git status
+ END
+ )
+'
+
+# this MUST be always the last test
+test_expect_success SUDO 'cleanup' '
+ sudo rm -rf root
+'
+
+test_done
|