dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
When using the dir_iterator API, we first stat(2) the base path, and then use that as a starting point to enumerate the directory's contents. If the directory contains symbolic links, we will immediately die() upon encountering them without the `FOLLOW_SYMLINKS` flag. The same is not true when resolving the top-level directory, though. As explained in a previous commit, this oversight in 6f054f9fb3 (builtin/clone.c: disallow `--local` clones with symlinks, 2022-07-28) can be used as an attack vector to include arbitrary files on a victim's filesystem from outside of the repository. Prevent resolving top-level symlinks unless the FOLLOW_SYMLINKS flag is given, which will cause clones of a repository with a symlink'd "$GIT_DIR/objects" directory to fail. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau committed
Jan 24, 2023 at 19:43 UTC
bffc762f87ae8d18c6001bf0044a76004245754c
4 files changed
+56
-5
dir-iterator.c
+9
-4
@@ -203,7 +203,7 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)
203
{
204
struct dir_iterator_int *iter = xcalloc(1, sizeof(*iter));
205
struct dir_iterator *dir_iterator = &iter->base;
206
- int saved_errno;
206
+ int saved_errno, err;
207
208
strbuf_init(&iter->base.path, PATH_MAX);
209
strbuf_addstr(&iter->base.path, path);
@@ -213,10 +213,15 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)
213
iter->flags = flags;
214
215
/*
216
- * Note: stat already checks for NULL or empty strings and
217
- * inexistent paths.
216
+ * Note: stat/lstat already checks for NULL or empty strings and
217
+ * nonexistent paths.
218
*/
219
- if (stat(iter->base.path.buf, &iter->base.st) < 0) {
219
+ if (iter->flags & DIR_ITERATOR_FOLLOW_SYMLINKS)
220
+ err = stat(iter->base.path.buf, &iter->base.st);
221
+ else
222
+ err = lstat(iter->base.path.buf, &iter->base.st);
223
+
224
+ if (err < 0) {
225
saved_errno = errno;
226
goto error_out;
227
}
dir-iterator.h
+5
@@ -61,6 +61,11 @@
61
* not the symlinks themselves, which is the default behavior. Broken
62
* symlinks are ignored.
63
*
64
+ * Note: setting DIR_ITERATOR_FOLLOW_SYMLINKS affects resolving the
65
+ * starting path as well (e.g., attempting to iterate starting at a
66
+ * symbolic link pointing to a directory without FOLLOW_SYMLINKS will
67
+ * result in an error).
68
+ *
69
* Warning: circular symlinks are also followed when
70
* DIR_ITERATOR_FOLLOW_SYMLINKS is set. The iteration may end up with
71
* an ELOOP if they happen and DIR_ITERATOR_PEDANTIC is set.
t/t0066-dir-iterator.sh
+26
-1
@@ -109,7 +109,9 @@ test_expect_success SYMLINKS 'setup dirs with symlinks' '
109
mkdir -p dir5/a/c &&
110
ln -s ../c dir5/a/b/d &&
111
ln -s ../ dir5/a/b/e &&
112
- ln -s ../../ dir5/a/b/f
112
+ ln -s ../../ dir5/a/b/f &&
113
+
114
+ ln -s dir4 dir6
115
'
116
117
test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default' '
@@ -145,4 +147,27 @@ test_expect_success SYMLINKS 'dir-iterator should follow symlinks w/ follow flag
147
test_cmp expected-follow-sorted-output actual-follow-sorted-output
148
'
149
150
+test_expect_success SYMLINKS 'dir-iterator does not resolve top-level symlinks' '
151
+ test_must_fail test-tool dir-iterator ./dir6 >out &&
152
+
153
+ grep "ENOTDIR" out
154
+'
155
+
156
+test_expect_success SYMLINKS 'dir-iterator resolves top-level symlinks w/ follow flag' '
157
+ cat >expected-follow-sorted-output <<-EOF &&
158
+ [d] (a) [a] ./dir6/a
159
+ [d] (a/f) [f] ./dir6/a/f
160
+ [d] (a/f/c) [c] ./dir6/a/f/c
161
+ [d] (b) [b] ./dir6/b
162
+ [d] (b/c) [c] ./dir6/b/c
163
+ [f] (a/d) [d] ./dir6/a/d
164
+ [f] (a/e) [e] ./dir6/a/e
165
+ EOF
166
+
167
+ test-tool dir-iterator --follow-symlinks ./dir6 >out &&
168
+ sort out >actual-follow-sorted-output &&
169
+
170
+ test_cmp expected-follow-sorted-output actual-follow-sorted-output
171
+'
172
+
173
test_done
t/t5604-clone-reference.sh
+16
@@ -341,4 +341,20 @@ test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at obje
341
test_must_be_empty T--shared.objects-symlinks.raw
342
'
343
344
+test_expect_success SYMLINKS 'clone repo with symlinked objects directory' '
345
+ test_when_finished "rm -fr sensitive malicious" &&
346
+
347
+ mkdir -p sensitive &&
348
+ echo "secret" >sensitive/file &&
349
+
350
+ git init malicious &&
351
+ rm -fr malicious/.git/objects &&
352
+ ln -s "$(pwd)/sensitive" ./malicious/.git/objects &&
353
+
354
+ test_must_fail git clone --local malicious clone 2>err &&
355
+
356
+ test_path_is_missing clone &&
357
+ grep "failed to start iterator over" err
358
+'
359
+
360
test_done