Browse code

git: Fix CVE-2021-21300

Change-Id: I99f21007ab8162c779bcce4e70a9ca5f8a0f1fcc
Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/12619
Reviewed-by: Alexey Makhalov <amakhalov@vmware.com>
Tested-by: gerrit-photon <photon-checkins@vmware.com>

Prashant Singh Chauhan authored on 2021/03/10 16:40:18
Showing 4 changed files
1 1
new file mode 100644
... ...
@@ -0,0 +1,311 @@
0
+From 684dd4c2b414bcf648505e74498a608f28de4592 Mon Sep 17 00:00:00 2001
1
+From: Matheus Tavares <matheus.bernardino@usp.br>
2
+Date: Thu, 10 Dec 2020 10:27:55 -0300
3
+Subject: [PATCH] checkout: fix bug that makes checkout follow symlinks in
4
+ leading path
5
+
6
+Before checking out a file, we have to confirm that all of its leading
7
+components are real existing directories. And to reduce the number of
8
+lstat() calls in this process, we cache the last leading path known to
9
+contain only directories. However, when a path collision occurs (e.g.
10
+when checking out case-sensitive files in case-insensitive file
11
+systems), a cached path might have its file type changed on disk,
12
+leaving the cache on an invalid state. Normally, this doesn't bring
13
+any bad consequences as we usually check out files in index order, and
14
+therefore, by the time the cached path becomes outdated, we no longer
15
+need it anyway (because all files in that directory would have already
16
+been written).
17
+
18
+But, there are some users of the checkout machinery that do not always
19
+follow the index order. In particular: checkout-index writes the paths
20
+in the same order that they appear on the CLI (or stdin); and the
21
+delayed checkout feature -- used when a long-running filter process
22
+replies with "status=delayed" -- postpones the checkout of some entries,
23
+thus modifying the checkout order.
24
+
25
+When we have to check out an out-of-order entry and the lstat() cache is
26
+invalid (due to a previous path collision), checkout_entry() may end up
27
+using the invalid data and thrusting that the leading components are
28
+real directories when, in reality, they are not. In the best case
29
+scenario, where the directory was replaced by a regular file, the user
30
+will get an error: "fatal: unable to create file 'foo/bar': Not a
31
+directory". But if the directory was replaced by a symlink, checkout
32
+could actually end up following the symlink and writing the file at a
33
+wrong place, even outside the repository. Since delayed checkout is
34
+affected by this bug, it could be used by an attacker to write
35
+arbitrary files during the clone of a maliciously crafted repository.
36
+
37
+Some candidate solutions considered were to disable the lstat() cache
38
+during unordered checkouts or sort the entries before passing them to
39
+the checkout machinery. But both ideas include some performance penalty
40
+and they don't future-proof the code against new unordered use cases.
41
+
42
+Instead, we now manually reset the lstat cache whenever we successfully
43
+remove a directory. Note: We are not even checking whether the directory
44
+was the same as the lstat cache points to because we might face a
45
+scenario where the paths refer to the same location but differ due to
46
+case folding, precomposed UTF-8 issues, or the presence of `..`
47
+components in the path. Two regression tests, with case-collisions and
48
+utf8-collisions, are also added for both checkout-index and delayed
49
+checkout.
50
+
51
+Note: to make the previously mentioned clone attack unfeasible, it would
52
+be sufficient to reset the lstat cache only after the remove_subtree()
53
+call inside checkout_entry(). This is the place where we would remove a
54
+directory whose path collides with the path of another entry that we are
55
+currently trying to check out (possibly a symlink). However, in the
56
+interest of a thorough fix that does not leave Git open to
57
+similar-but-not-identical attack vectors, we decided to intercept
58
+all `rmdir()` calls in one fell swoop.
59
+
60
+This addresses CVE-2021-21300.
61
+
62
+Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de>
63
+Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
64
+---
65
+ cache.h                         |  1 +
66
+ compat/mingw.c                  |  2 ++
67
+ git-compat-util.h               |  5 ++++
68
+ symlinks.c                      | 24 +++++++++++++++++
69
+ t/t0021-conversion.sh           | 45 ++++++++++++++++++++++++++++++++
70
+ t/t0021/rot13-filter.pl         | 21 ++++++++++++---
71
+ t/t2006-checkout-index-basic.sh | 46 +++++++++++++++++++++++++++++++++
72
+ 7 files changed, 141 insertions(+), 3 deletions(-)
73
+
74
+diff --git a/cache.h b/cache.h
75
+index 0323853c99..c530593971 100644
76
+--- a/cache.h
77
+@@ -1631,6 +1631,7 @@ int has_symlink_leading_path(const char *name, int len);
78
+ int threaded_has_symlink_leading_path(struct cache_def *, const char *, int);
79
+ int check_leading_path(const char *name, int len);
80
+ int has_dirs_only_path(const char *name, int len, int prefix_len);
81
++void invalidate_lstat_cache(void);
82
+ void schedule_dir_for_removal(const char *name, int len);
83
+ void remove_scheduled_dirs(void);
84
+ 
85
+diff --git a/compat/mingw.c b/compat/mingw.c
86
+index b047e2166096f..0c414d08b69aa 100644
87
+--- a/compat/mingw.c
88
+@@ -340,6 +340,8 @@ int mingw_rmdir(const char *pathname)
89
+ 	       ask_yes_no_if_possible("Deletion of directory '%s' failed. "
90
+ 			"Should I try again?", pathname))
91
+ 	       ret = _wrmdir(wpathname);
92
++	if (!ret)
93
++		invalidate_lstat_cache();
94
+ 	return ret;
95
+ }
96
+ 
97
+diff --git a/git-compat-util.h b/git-compat-util.h
98
+index 37277494f9..6230f9aaf3 100644
99
+--- a/git-compat-util.h
100
+@@ -364,6 +364,11 @@ static inline int noop_core_config(const char *var, const char *value, void *cb)
101
+ #define platform_core_config noop_core_config
102
+ #endif
103
+ 
104
++int lstat_cache_aware_rmdir(const char *path);
105
++#if !defined(__MINGW32__) && !defined(_MSC_VER)
106
++#define rmdir lstat_cache_aware_rmdir
107
++#endif
108
++
109
+ #ifndef has_dos_drive_prefix
110
+ static inline int git_has_dos_drive_prefix(const char *path)
111
+ {
112
+diff --git a/symlinks.c b/symlinks.c
113
+index 5261e8cf49..53b770be08 100644
114
+--- a/symlinks.c
115
+@@ -267,6 +267,13 @@ int has_dirs_only_path(const char *name, int len, int prefix_len)
116
+  */
117
+ static int threaded_has_dirs_only_path(struct cache_def *cache, const char *name, int len, int prefix_len)
118
+ {
119
++	/*
120
++	 * Note: this function is used by the checkout machinery, which also
121
++	 * takes care to properly reset the cache when it performs an operation
122
++	 * that would leave the cache outdated. If this function starts caching
123
++	 * anything else besides FL_DIR, remember to also invalidate the cache
124
++	 * when creating or deleting paths that might be in the cache.
125
++	 */
126
+ 	return lstat_cache(cache, name, len,
127
+ 			   FL_DIR|FL_FULLPATH, prefix_len) &
128
+ 		FL_DIR;
129
+@@ -321,3 +328,20 @@ void remove_scheduled_dirs(void)
130
+ {
131
+ 	do_remove_scheduled_dirs(0);
132
+ }
133
++
134
++void invalidate_lstat_cache(void)
135
++{
136
++	reset_lstat_cache(&default_cache);
137
++}
138
++
139
++#undef rmdir
140
++int lstat_cache_aware_rmdir(const char *path)
141
++{
142
++	/* Any change in this function must be made also in `mingw_rmdir()` */
143
++	int ret = rmdir(path);
144
++
145
++	if (!ret)
146
++		invalidate_lstat_cache();
147
++
148
++	return ret;
149
++}
150
+diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
151
+index 46f8e583c37da..8ff917fca6d9f 100755
152
+--- a/t/t0021-conversion.sh
153
+@@ -817,4 +817,49 @@ test_expect_success PERL 'invalid file in delayed checkout' '
154
+ 	grep "error: external filter .* signaled that .unfiltered. is now available although it has not been delayed earlier" git-stderr.log
155
+ '
156
+ 
157
++for mode in 'case' 'utf-8'
158
++do
159
++	case "$mode" in
160
++	case)	dir='A' symlink='a' mode_prereq='CASE_INSENSITIVE_FS' ;;
161
++	utf-8)
162
++		dir=$(printf "\141\314\210") symlink=$(printf "\303\244")
163
++		mode_prereq='UTF8_NFD_TO_NFC' ;;
164
++	esac
165
++
166
++	test_expect_success PERL,SYMLINKS,$mode_prereq \
167
++	"delayed checkout with $mode-collision don't write to the wrong place" '
168
++		test_config_global filter.delay.process \
169
++			"\"$TEST_ROOT/rot13-filter.pl\" --always-delay delayed.log clean smudge delay" &&
170
++		test_config_global filter.delay.required true &&
171
++
172
++		git init $mode-collision &&
173
++		(
174
++			cd $mode-collision &&
175
++			mkdir target-dir &&
176
++
177
++			empty_oid=$(printf "" | git hash-object -w --stdin) &&
178
++			symlink_oid=$(printf "%s" "$PWD/target-dir" | git hash-object -w --stdin) &&
179
++			attr_oid=$(echo "$dir/z filter=delay" | git hash-object -w --stdin) &&
180
++
181
++			cat >objs <<-EOF &&
182
++			100644 blob $empty_oid	$dir/x
183
++			100644 blob $empty_oid	$dir/y
184
++			100644 blob $empty_oid	$dir/z
185
++			120000 blob $symlink_oid	$symlink
186
++			100644 blob $attr_oid	.gitattributes
187
++			EOF
188
++
189
++			git update-index --index-info <objs &&
190
++			git commit -m "test commit"
191
++		) &&
192
++
193
++		git clone $mode-collision $mode-collision-cloned &&
194
++		# Make sure z was really delayed
195
++		grep "IN: smudge $dir/z .* \\[DELAYED\\]" $mode-collision-cloned/delayed.log &&
196
++
197
++		# Should not create $dir/z at $symlink/z
198
++		test_path_is_missing $mode-collision/target-dir/z
199
++	'
200
++done
201
++
202
+ test_done
203
+diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
204
+index 470107248eb16..007f2d78ea5b0 100644
205
+--- a/t/t0021/rot13-filter.pl
206
+@@ -2,9 +2,15 @@
207
+ # Example implementation for the Git filter protocol version 2
208
+ # See Documentation/gitattributes.txt, section "Filter Protocol"
209
+ #
210
+-# The first argument defines a debug log file that the script write to.
211
+-# All remaining arguments define a list of supported protocol
212
+-# capabilities ("clean", "smudge", etc).
213
++# Usage: rot13-filter.pl [--always-delay] <log path> <capabilities>
214
++#
215
++# Log path defines a debug log file that the script writes to. The
216
++# subsequent arguments define a list of supported protocol capabilities
217
++# ("clean", "smudge", etc).
218
++#
219
++# When --always-delay is given all pathnames with the "can-delay" flag
220
++# that don't appear on the list bellow are delayed with a count of 1
221
++# (see more below).
222
+ #
223
+ # This implementation supports special test cases:
224
+ # (1) If data with the pathname "clean-write-fail.r" is processed with
225
+@@ -53,6 +59,13 @@ sub gitperllib {
226
+ use Git::Packet;
227
+ 
228
+ my $MAX_PACKET_CONTENT_SIZE = 65516;
229
++
230
++my $always_delay = 0;
231
++if ( $ARGV[0] eq '--always-delay' ) {
232
++	$always_delay = 1;
233
++	shift @ARGV;
234
++}
235
++
236
+ my $log_file                = shift @ARGV;
237
+ my @capabilities            = @ARGV;
238
+ 
239
+@@ -134,6 +147,8 @@ sub rot13 {
240
+ 			if ( $buffer eq "can-delay=1" ) {
241
+ 				if ( exists $DELAY{$pathname} and $DELAY{$pathname}{"requested"} == 0 ) {
242
+ 					$DELAY{$pathname}{"requested"} = 1;
243
++				} elsif ( !exists $DELAY{$pathname} and $always_delay ) {
244
++					$DELAY{$pathname} = { "requested" => 1, "count" => 1 };
245
+ 				}
246
+ 			} elsif ($buffer =~ /^(ref|treeish|blob)=/) {
247
+ 				print $debug " $buffer";
248
+diff --git a/t/t2006-checkout-index-basic.sh b/t/t2006-checkout-index-basic.sh
249
+index 8e181db..a95dcf3 100755
250
+--- a/t/t2006-checkout-index-basic.sh
251
+@@ -21,6 +21,52 @@ test_expect_success 'checkout-index -h in broken repository' '
252
+ 	test_i18ngrep "[Uu]sage" broken/usage
253
+ '
254
+ 
255
++for mode in 'case' 'utf-8'
256
++do
257
++	case "$mode" in
258
++	case)	dir='A' symlink='a' mode_prereq='CASE_INSENSITIVE_FS' ;;
259
++	utf-8)
260
++		dir=$(printf "\141\314\210") symlink=$(printf "\303\244")
261
++		mode_prereq='UTF8_NFD_TO_NFC' ;;
262
++	esac
263
++
264
++	test_expect_success SYMLINKS,$mode_prereq \
265
++	"checkout-index with $mode-collision don't write to the wrong place" '
266
++		git init $mode-collision &&
267
++		(
268
++			cd $mode-collision &&
269
++			mkdir target-dir &&
270
++
271
++			empty_obj_hex=$(git hash-object -w --stdin </dev/null) &&
272
++			symlink_hex=$(printf "%s" "$PWD/target-dir" | git hash-object -w --stdin) &&
273
++
274
++			cat >objs <<-EOF &&
275
++			100644 blob ${empty_obj_hex}	${dir}/x
276
++			100644 blob ${empty_obj_hex}	${dir}/y
277
++			100644 blob ${empty_obj_hex}	${dir}/z
278
++			120000 blob ${symlink_hex}	${symlink}
279
++			EOF
280
++
281
++			git update-index --index-info <objs &&
282
++
283
++			# Note: the order is important here to exercise the
284
++			# case where the file at ${dir} has its type changed by
285
++			# the time Git tries to check out ${dir}/z.
286
++			#
287
++			# Also, we use core.precomposeUnicode=false because we
288
++			# want Git to treat the UTF-8 paths transparently on
289
++			# Mac OS, matching what is in the index.
290
++			#
291
++			git -c core.precomposeUnicode=false checkout-index -f \
292
++				${dir}/x ${dir}/y ${symlink} ${dir}/z &&
293
++
294
++			# Should not create ${dir}/z at ${symlink}/z
295
++			test_path_is_missing target-dir/z
296
++
297
++		)
298
++	'
299
++done
300
++
301
+ test_expect_success 'checkout-index reports errors (cmdline)' '
302
+ 	test_must_fail git checkout-index -- does-not-exist 2>stderr &&
303
+ 	test_i18ngrep not.in.the.cache stderr
0 304
new file mode 100644
... ...
@@ -0,0 +1,106 @@
0
+From 0d58fef58a6f382ba1d35f47a01cb55d8976335f Mon Sep 17 00:00:00 2001
1
+From: Johannes Schindelin <johannes.schindelin@gmx.de>
2
+Date: Tue, 2 Feb 2021 22:09:52 +0100
3
+Subject: [PATCH] run-command: invalidate lstat cache after a command finished
4
+
5
+In the previous commit, we intercepted calls to `rmdir()` to invalidate
6
+the lstat cache in the successful case, so that the lstat cache could
7
+not have the idea that a directory exists where there is none.
8
+
9
+The same situation can arise, of course, when a separate process is
10
+spawned (most notably, this is the case in `submodule_move_head()`).
11
+Obviously, we cannot know whether a directory was removed in that
12
+process, therefore we must invalidate the lstat cache afterwards.
13
+
14
+Note: in contrast to `lstat_cache_aware_rmdir()`, we invalidate the
15
+lstat cache even in case of an error: the process might have removed a
16
+directory and still have failed afterwards.
17
+
18
+Co-authored-by: Matheus Tavares <matheus.bernardino@usp.br>
19
+Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
20
+---
21
+ run-command.c         |  9 ++++++++-
22
+ t/t0021-conversion.sh | 36 ++++++++++++++++++++++++++++++++++++
23
+ 2 files changed, 44 insertions(+), 1 deletion(-)
24
+
25
+diff --git a/run-command.c b/run-command.c
26
+index a483d5904a..c5c4d36671 100644
27
+--- a/run-command.c
28
+@@ -989,6 +989,7 @@ int finish_command(struct child_process *cmd)
29
+ 	int ret = wait_or_whine(cmd->pid, cmd->argv[0], 0);
30
+ 	trace2_child_exit(cmd, ret);
31
+ 	child_process_clear(cmd);
32
++	invalidate_lstat_cache();
33
+ 	return ret;
34
+ }
35
+ 
36
+@@ -1239,13 +1240,19 @@ int start_async(struct async *async)
37
+ int finish_async(struct async *async)
38
+ {
39
+ #ifdef NO_PTHREADS
40
+-	return wait_or_whine(async->pid, "child process", 0);
41
++	int ret = wait_or_whine(async->pid, "child process", 0);
42
++
43
++	invalidate_lstat_cache();
44
++
45
++	return ret;
46
+ #else
47
+ 	void *ret = (void *)(intptr_t)(-1);
48
+ 
49
+ 	if (pthread_join(async->tid, &ret))
50
+ 		error("pthread_join failed");
51
++	invalidate_lstat_cache();
52
+ 	return (int)(intptr_t)ret;
53
++
54
+ #endif
55
+ }
56
+ 
57
+diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
58
+index 8ff917fca6..a714d376a3 100755
59
+--- a/t/t0021-conversion.sh
60
+@@ -862,4 +862,40 @@ do
61
+ 	'
62
+ done
63
+ 
64
++test_expect_success PERL,SYMLINKS,CASE_INSENSITIVE_FS \
65
++"delayed checkout with submodule collision don't write to the wrong place" '
66
++	git init collision-with-submodule &&
67
++	(
68
++		cd collision-with-submodule &&
69
++		git config filter.delay.process "\"$TEST_ROOT/rot13-filter.pl\" --always-delay delayed.log clean smudge delay" &&
70
++		git config filter.delay.required true &&
71
++
72
++		# We need Git to treat the submodule "a" and the
73
++		# leading dir "A" as different paths in the index.
74
++		git config --local core.ignoreCase false &&
75
++
76
++		empty_oid=$(printf "" | git hash-object -w --stdin) &&
77
++		attr_oid=$(echo "A/B/y filter=delay" | git hash-object -w --stdin) &&
78
++		cat >objs <<-EOF &&
79
++		100644 blob $empty_oid	A/B/x
80
++		100644 blob $empty_oid	A/B/y
81
++		100644 blob $attr_oid	.gitattributes
82
++		EOF
83
++		git update-index --index-info <objs &&
84
++
85
++		git init a &&
86
++		mkdir target-dir &&
87
++		symlink_oid=$(printf "%s" "$PWD/target-dir" | git -C a hash-object -w --stdin) &&
88
++		echo "120000 blob $symlink_oid	b" >objs &&
89
++		git -C a update-index --index-info <objs &&
90
++		git -C a commit -m sub &&
91
++		git submodule add ./a &&
92
++		git commit -m super &&
93
++
94
++		git checkout --recurse-submodules . &&
95
++		grep "IN: smudge A/B/y .* \\[DELAYED\\]" delayed.log &&
96
++		test_path_is_missing target-dir/y
97
++	)
98
++'
99
++
100
+ test_done
101
+-- 
102
+2.30.0
103
+
0 104
new file mode 100644
... ...
@@ -0,0 +1,27 @@
0
+From 22539ec3b5e678c054ab361a37a7cdcc64ca1228 Mon Sep 17 00:00:00 2001
1
+From: Matheus Tavares <matheus.bernardino@usp.br>
2
+Date: Tue, 2 Feb 2021 22:37:10 +0100
3
+Subject: [PATCH] unpack_trees(): start with a fresh lstat cache
4
+
5
+We really want to avoid relying on stale information.
6
+
7
+Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
8
+Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
9
+---
10
+ unpack-trees.c | 3 +++
11
+ 1 file changed, 3 insertions(+)
12
+
13
+diff --git a/unpack-trees.c b/unpack-trees.c
14
+index 323280d..2344b5e 100644
15
+--- a/unpack-trees.c
16
+@@ -417,6 +417,9 @@ static int check_updates(struct unpack_trees_options *o,
17
+ 
18
+ 	progress = get_progress(o, index);
19
+ 
20
++	/* Start with clean cache to avoid using any possibly outdated info. */
21
++	invalidate_lstat_cache();
22
++
23
+ 	git_attr_set_direction(GIT_ATTR_CHECKOUT);
24
+ 
25
+ 	if (should_update_submodules())
... ...
@@ -1,7 +1,7 @@
1 1
 Summary:        Fast distributed version control system
2 2
 Name:           git
3 3
 Version:        2.30.0
4
-Release:        2%{?dist}
4
+Release:        3%{?dist}
5 5
 License:        GPLv2
6 6
 URL:            http://git-scm.com/
7 7
 Group:          System Environment/Programming
... ...
@@ -9,6 +9,9 @@ Vendor:         VMware, Inc.
9 9
 Distribution:   Photon
10 10
 Source0:        https://www.kernel.org/pub/software/scm/git/%{name}-%{version}.tar.xz
11 11
 %define sha1    git=6be02a878d08227d85f0cf4d5646b19c60a242e4
12
+Patch0:         CVE-2021-21300-1.patch
13
+Patch1:         CVE-2021-21300-2.patch
14
+Patch2:         CVE-2021-21300-3.patch
12 15
 BuildRequires:  curl-devel
13 16
 BuildRequires:  python3
14 17
 BuildRequires:  python3-devel
... ...
@@ -44,6 +47,9 @@ These are the additional language files of git.
44 44
 
45 45
 %prep
46 46
 %setup -q
47
+%patch0 -p1
48
+%patch1 -p1
49
+%patch2 -p1
47 50
 %build
48 51
 %configure \
49 52
     CFLAGS="%{optflags}" \
... ...
@@ -94,6 +100,8 @@ rm -rf %{buildroot}/*
94 94
 %defattr(-,root,root)
95 95
 
96 96
 %changelog
97
+*   Tue Mar 09 2021 Prashant S Chauhan <psinghchauha@vmware.com> 2.30.0-3
98
+-   Fix CVE-2021-21300
97 99
 *   Mon Feb 01 2021 Shreenidhi Shedi <sshedi@vmware.com> 2.30.0-2
98 100
 -   Fix build with new rpm
99 101
 *   Sat Jan 23 2021 Susant Sahani <ssahani@vmware.com> 2.30.0-1
... ...
@@ -110,7 +118,7 @@ rm -rf %{buildroot}/*
110 110
 -   Added patch, Fixes CVE-2020-5260
111 111
 *   Wed Apr 01 2020 Susant Sahani <ssahani@vmware.com> 2.26.0-1
112 112
 -   Updated to version 2.26.0
113
-*   Wed Feb 12 2019 Prashant S Chauhan <psinghchauha@vmware.com> 2.23.1-1
113
+*   Tue Feb 12 2019 Prashant S Chauhan <psinghchauha@vmware.com> 2.23.1-1
114 114
 -   Updated to version 2.23.1 . Fixes CVE-2019-1348
115 115
 *   Thu Jan 10 2019 Alexey Makhalov <amakhalov@vmware.com> 2.19.0-3
116 116
 -   Added Requires python2