151 lines
5.1 KiB
Diff
151 lines
5.1 KiB
Diff
From bffc762f87ae8d18c6001bf0044a76004245754c Mon Sep 17 00:00:00 2001
|
|
From: Taylor Blau <me@ttaylorr.com>
|
|
Date: Tue, 24 Jan 2023 19:43:51 -0500
|
|
Subject: [PATCH] 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>
|
|
---
|
|
dir-iterator.c | 13 +++++++++----
|
|
dir-iterator.h | 5 +++++
|
|
t/t0066-dir-iterator.sh | 27 ++++++++++++++++++++++++++-
|
|
t/t5604-clone-reference.sh | 16 ++++++++++++++++
|
|
4 files changed, 56 insertions(+), 5 deletions(-)
|
|
|
|
diff --git a/dir-iterator.c b/dir-iterator.c
|
|
index b17e9f970a..3764dd81a1 100644
|
|
--- a/dir-iterator.c
|
|
+++ b/dir-iterator.c
|
|
@@ -203,7 +203,7 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)
|
|
{
|
|
struct dir_iterator_int *iter = xcalloc(1, sizeof(*iter));
|
|
struct dir_iterator *dir_iterator = &iter->base;
|
|
- int saved_errno;
|
|
+ int saved_errno, err;
|
|
|
|
strbuf_init(&iter->base.path, PATH_MAX);
|
|
strbuf_addstr(&iter->base.path, path);
|
|
@@ -213,10 +213,15 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)
|
|
iter->flags = flags;
|
|
|
|
/*
|
|
- * Note: stat already checks for NULL or empty strings and
|
|
- * inexistent paths.
|
|
+ * Note: stat/lstat already checks for NULL or empty strings and
|
|
+ * nonexistent paths.
|
|
*/
|
|
- if (stat(iter->base.path.buf, &iter->base.st) < 0) {
|
|
+ if (iter->flags & DIR_ITERATOR_FOLLOW_SYMLINKS)
|
|
+ err = stat(iter->base.path.buf, &iter->base.st);
|
|
+ else
|
|
+ err = lstat(iter->base.path.buf, &iter->base.st);
|
|
+
|
|
+ if (err < 0) {
|
|
saved_errno = errno;
|
|
goto error_out;
|
|
}
|
|
diff --git a/dir-iterator.h b/dir-iterator.h
|
|
index 08229157c6..e3b6ff2800 100644
|
|
--- a/dir-iterator.h
|
|
+++ b/dir-iterator.h
|
|
@@ -61,6 +61,11 @@
|
|
* not the symlinks themselves, which is the default behavior. Broken
|
|
* symlinks are ignored.
|
|
*
|
|
+ * Note: setting DIR_ITERATOR_FOLLOW_SYMLINKS affects resolving the
|
|
+ * starting path as well (e.g., attempting to iterate starting at a
|
|
+ * symbolic link pointing to a directory without FOLLOW_SYMLINKS will
|
|
+ * result in an error).
|
|
+ *
|
|
* Warning: circular symlinks are also followed when
|
|
* DIR_ITERATOR_FOLLOW_SYMLINKS is set. The iteration may end up with
|
|
* an ELOOP if they happen and DIR_ITERATOR_PEDANTIC is set.
|
|
diff --git a/t/t0066-dir-iterator.sh b/t/t0066-dir-iterator.sh
|
|
index 92910e4e6c..c826f60f6d 100755
|
|
--- a/t/t0066-dir-iterator.sh
|
|
+++ b/t/t0066-dir-iterator.sh
|
|
@@ -109,7 +109,9 @@ test_expect_success SYMLINKS 'setup dirs with symlinks' '
|
|
mkdir -p dir5/a/c &&
|
|
ln -s ../c dir5/a/b/d &&
|
|
ln -s ../ dir5/a/b/e &&
|
|
- ln -s ../../ dir5/a/b/f
|
|
+ ln -s ../../ dir5/a/b/f &&
|
|
+
|
|
+ ln -s dir4 dir6
|
|
'
|
|
|
|
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
|
|
test_cmp expected-follow-sorted-output actual-follow-sorted-output
|
|
'
|
|
|
|
+test_expect_success SYMLINKS 'dir-iterator does not resolve top-level symlinks' '
|
|
+ test_must_fail test-tool dir-iterator ./dir6 >out &&
|
|
+
|
|
+ grep "ENOTDIR" out
|
|
+'
|
|
+
|
|
+test_expect_success SYMLINKS 'dir-iterator resolves top-level symlinks w/ follow flag' '
|
|
+ cat >expected-follow-sorted-output <<-EOF &&
|
|
+ [d] (a) [a] ./dir6/a
|
|
+ [d] (a/f) [f] ./dir6/a/f
|
|
+ [d] (a/f/c) [c] ./dir6/a/f/c
|
|
+ [d] (b) [b] ./dir6/b
|
|
+ [d] (b/c) [c] ./dir6/b/c
|
|
+ [f] (a/d) [d] ./dir6/a/d
|
|
+ [f] (a/e) [e] ./dir6/a/e
|
|
+ EOF
|
|
+
|
|
+ test-tool dir-iterator --follow-symlinks ./dir6 >out &&
|
|
+ sort out >actual-follow-sorted-output &&
|
|
+
|
|
+ test_cmp expected-follow-sorted-output actual-follow-sorted-output
|
|
+'
|
|
+
|
|
test_done
|
|
diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
|
|
index 9d32f1c4a4..4ff21d7ccf 100755
|
|
--- a/t/t5604-clone-reference.sh
|
|
+++ b/t/t5604-clone-reference.sh
|
|
@@ -341,4 +341,20 @@ test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at obje
|
|
test_must_be_empty T--shared.objects-symlinks.raw
|
|
'
|
|
|
|
+test_expect_success SYMLINKS 'clone repo with symlinked objects directory' '
|
|
+ test_when_finished "rm -fr sensitive malicious" &&
|
|
+
|
|
+ mkdir -p sensitive &&
|
|
+ echo "secret" >sensitive/file &&
|
|
+
|
|
+ git init malicious &&
|
|
+ rm -fr malicious/.git/objects &&
|
|
+ ln -s "$(pwd)/sensitive" ./malicious/.git/objects &&
|
|
+
|
|
+ test_must_fail git clone --local malicious clone 2>err &&
|
|
+
|
|
+ test_path_is_missing clone &&
|
|
+ grep "failed to start iterator over" err
|
|
+'
|
|
+
|
|
test_done
|
|
--
|
|
2.27.0
|
|
|