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
|
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 3/3] 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 6f054f9
(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>
Upstream-Status: Backport
[https://github.com/git/git/commit/bffc762f87ae8d18c6001bf0044a76004245754c]
CVE: CVE-2023-22490
Signed-off-by: Vijay Anusuri <vanusuri@mvista.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 b17e9f9..3764dd8 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 0822915..e3b6ff2 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 92910e4..c826f60 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 4894237..615b981 100755
--- a/t/t5604-clone-reference.sh
+++ b/t/t5604-clone-reference.sh
@@ -354,4 +354,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.25.1
|
