list-objects-filter: disable 'sparse:path' filters

If someone wants to use as a filter a sparse file that is in the repository, something like "--filter=sparse:oid=<ref>:<path>" already works. So 'sparse:path' is only interesting if the sparse file is not in the repository. In this case though the current implementation has a big security issue, as it makes it possible to ask the server to read any file, like for example /etc/password, and to explore the filesystem, as well as individual lines of files. If someone is interested in using a sparse file that is not in the repository as a filter, then at the minimum a config option, such as "uploadpack.sparsePathFilter", should be implemented first to restrict the directory from which the files specified by 'sparse:path' can be read. For now though, let's just disable 'sparse:path' filters. Helped-by: Matthew DeVore <matvore@google.com> Helped-by: Jeff Hostetler <git@jeffhostetler.com> Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Christian Couder committed May 29, 2019 at 14:44 UTC e693237e2ba27b6129e8af7f6a794f5c2fbd26f3
7 files changed +37 -116
Documentation/rev-list-options.txt
+4 -3
@@ -725,9 +725,6 @@ specification contained in the blob (or blob-expression) '<blob-ish>'
725 to omit blobs that would not be not required for a sparse checkout on
726 the requested refs.
727 +
728 -The form '--filter=sparse:path=<path>' similarly uses a sparse-checkout
729 -specification contained in <path>.
730 -+
728 The form '--filter=tree:<depth>' omits all blobs and trees whose depth
729 from the root tree is >= <depth> (minimum depth if an object is located
730 at multiple depths in the commits traversed). <depth>=0 will not include
@@ -737,6 +734,10 @@ tree and blobs which are referenced directly by a commit reachable from
734 <commit> or an explicitly-given object. <depth>=2 is like <depth>=1
735 while also including trees and blobs one more level removed from an
736 explicitly-given commit or tree.
737 ++
738 +Note that the form '--filter=sparse:path=<path>' that wants to read
739 +from an arbitrary path on the filesystem has been dropped for security
740 +reasons.
741
742 --no-filter::
743 Turn off any previous `--filter=` argument.
contrib/completion/git-completion.bash
+1 -1
@@ -1536,7 +1536,7 @@ _git_fetch ()
1536 return
1537 ;;
1538 --filter=*)
1539 - __gitcomp "blob:none blob:limit= sparse:oid= sparse:path=" "" "${cur##--filter=}"
1539 + __gitcomp "blob:none blob:limit= sparse:oid=" "" "${cur##--filter=}"
1540 return
1541 ;;
1542 --*)
list-objects-filter-options.c
+6 -4
@@ -78,9 +78,12 @@ static int gently_parse_list_objects_filter(
78 return 0;
79
80 } else if (skip_prefix(arg, "sparse:path=", &v0)) {
81 - filter_options->choice = LOFC_SPARSE_PATH;
82 - filter_options->sparse_path_value = strdup(v0);
83 - return 0;
81 + if (errbuf) {
82 + strbuf_addstr(
83 + errbuf,
84 + _("sparse:path filters support has been dropped"));
85 + }
86 + return 1;
87 }
88 /*
89 * Please update _git_fetch() in git-completion.bash when you
@@ -136,7 +139,6 @@ void list_objects_filter_release(
139 {
140 free(filter_options->filter_spec);
141 free(filter_options->sparse_oid_value);
139 - free(filter_options->sparse_path_value);
142 memset(filter_options, 0, sizeof(*filter_options));
143 }
144
list-objects-filter-options.h
-2
@@ -13,7 +13,6 @@ enum list_objects_filter_choice {
13 LOFC_BLOB_LIMIT,
14 LOFC_TREE_DEPTH,
15 LOFC_SPARSE_OID,
16 - LOFC_SPARSE_PATH,
16 LOFC__COUNT /* must be last */
17 };
18
@@ -44,7 +43,6 @@ struct list_objects_filter_options {
43 * choice.
44 */
45 struct object_id *sparse_oid_value;
47 - char *sparse_path_value;
46 unsigned long blob_limit_value;
47 unsigned long tree_exclude_depth;
48 };
list-objects-filter.c
-22
@@ -478,27 +478,6 @@ static void *filter_sparse_oid__init(
478 return d;
479 }
480
481 -static void *filter_sparse_path__init(
482 - struct oidset *omitted,
483 - struct list_objects_filter_options *filter_options,
484 - filter_object_fn *filter_fn,
485 - filter_free_fn *filter_free_fn)
486 -{
487 - struct filter_sparse_data *d = xcalloc(1, sizeof(*d));
488 - d->omits = omitted;
489 - if (add_excludes_from_file_to_list(filter_options->sparse_path_value,
490 - NULL, 0, &d->el, NULL) < 0)
491 - die("could not load filter specification");
492 -
493 - ALLOC_GROW(d->array_frame, d->nr + 1, d->alloc);
494 - d->array_frame[d->nr].defval = 0; /* default to include */
495 - d->array_frame[d->nr].child_prov_omit = 0;
496 -
497 - *filter_fn = filter_sparse;
498 - *filter_free_fn = filter_sparse_free;
499 - return d;
500 -}
501 -
481 typedef void *(*filter_init_fn)(
482 struct oidset *omitted,
483 struct list_objects_filter_options *filter_options,
@@ -514,7 +493,6 @@ static filter_init_fn s_filters[] = {
493 filter_blobs_limit__init,
494 filter_trees_depth__init,
495 filter_sparse_oid__init,
517 - filter_sparse_path__init,
496 };
497
498 void *list_objects_filter__init(
t/t5317-pack-objects-filter-objects.sh
+13 -58
@@ -277,6 +277,10 @@ test_expect_success 'verify normal and blob:limit packfiles have same commits/tr
277 '
278
279 # Test sparse:path=<path> filter.
280 +# !!!!
281 +# NOTE: sparse:path filter support has been dropped for security reasons,
282 +# so the tests have been changed to make sure that using it fails.
283 +# !!!!
284 # Use a local file containing a sparse-checkout specification to filter
285 # out blobs not required for the corresponding sparse-checkout. We do not
286 # require sparse-checkout to actually be enabled.
@@ -315,73 +319,24 @@ test_expect_success 'verify blob count in normal packfile' '
319 test_cmp expected observed
320 '
321
318 -test_expect_success 'verify sparse:path=pattern1' '
319 - git -C r3 ls-files -s dir1/sparse1 dir1/sparse2 >ls_files_result &&
320 - awk -f print_2.awk ls_files_result |
321 - sort >expected &&
322 -
323 - git -C r3 pack-objects --revs --stdout --filter=sparse:path=../pattern1 >filter.pack <<-EOF &&
322 +test_expect_success 'verify sparse:path=pattern1 fails' '
323 + test_must_fail git -C r3 pack-objects --revs --stdout \
324 + --filter=sparse:path=../pattern1 <<-EOF
325 HEAD
326 EOF
326 - git -C r3 index-pack ../filter.pack &&
327 -
328 - git -C r3 verify-pack -v ../filter.pack >verify_result &&
329 - grep blob verify_result |
330 - awk -f print_1.awk |
331 - sort >observed &&
332 -
333 - test_cmp expected observed
334 -'
335 -
336 -test_expect_success 'verify normal and sparse:path=pattern1 packfiles have same commits/trees' '
337 - git -C r3 verify-pack -v ../all.pack >verify_result &&
338 - grep -E "commit|tree" verify_result |
339 - awk -f print_1.awk |
340 - sort >expected &&
341 -
342 - git -C r3 verify-pack -v ../filter.pack >verify_result &&
343 - grep -E "commit|tree" verify_result |
344 - awk -f print_1.awk |
345 - sort >observed &&
346 -
347 - test_cmp expected observed
327 '
328
350 -test_expect_success 'verify sparse:path=pattern2' '
351 - git -C r3 ls-files -s sparse1 dir1/sparse1 >ls_files_result &&
352 - awk -f print_2.awk ls_files_result |
353 - sort >expected &&
354 -
355 - git -C r3 pack-objects --revs --stdout --filter=sparse:path=../pattern2 >filter.pack <<-EOF &&
329 +test_expect_success 'verify sparse:path=pattern2 fails' '
330 + test_must_fail git -C r3 pack-objects --revs --stdout \
331 + --filter=sparse:path=../pattern2 <<-EOF
332 HEAD
333 EOF
358 - git -C r3 index-pack ../filter.pack &&
359 -
360 - git -C r3 verify-pack -v ../filter.pack >verify_result &&
361 - grep blob verify_result |
362 - awk -f print_1.awk |
363 - sort >observed &&
364 -
365 - test_cmp expected observed
366 -'
367 -
368 -test_expect_success 'verify normal and sparse:path=pattern2 packfiles have same commits/trees' '
369 - git -C r3 verify-pack -v ../all.pack >verify_result &&
370 - grep -E "commit|tree" verify_result |
371 - awk -f print_1.awk |
372 - sort >expected &&
373 -
374 - git -C r3 verify-pack -v ../filter.pack >verify_result &&
375 - grep -E "commit|tree" verify_result |
376 - awk -f print_1.awk |
377 - sort >observed &&
378 -
379 - test_cmp expected observed
334 '
335
336 # Test sparse:oid=<oid-ish> filter.
383 -# Like sparse:path, but we get the sparse-checkout specification from
384 -# a blob rather than a file on disk.
337 +# Use a blob containing a sparse-checkout specification to filter
338 +# out blobs not required for the corresponding sparse-checkout. We do not
339 +# require sparse-checkout to actually be enabled.
340
341 test_expect_success 'setup r4' '
342 git init r4 &&
t/t6112-rev-list-filters-objects.sh
+13 -26
@@ -157,6 +157,10 @@ test_expect_success 'verify blob:limit=1m' '
157 '
158
159 # Test sparse:path=<path> filter.
160 +# !!!!
161 +# NOTE: sparse:path filter support has been dropped for security reasons,
162 +# so the tests have been changed to make sure that using it fails.
163 +# !!!!
164 # Use a local file containing a sparse-checkout specification to filter
165 # out blobs not required for the corresponding sparse-checkout. We do not
166 # require sparse-checkout to actually be enabled.
@@ -176,37 +180,20 @@ test_expect_success 'setup r3' '
180 echo sparse1 >pattern2
181 '
182
179 -test_expect_success 'verify sparse:path=pattern1 omits top-level files' '
180 - git -C r3 ls-files -s sparse1 sparse2 >ls_files_result &&
181 - awk -f print_2.awk ls_files_result |
182 - sort >expected &&
183 -
184 - git -C r3 rev-list --quiet --objects --filter-print-omitted \
185 - --filter=sparse:path=../pattern1 HEAD >revs &&
186 - awk -f print_1.awk revs |
187 - sed "s/~//" |
188 - sort >observed &&
189 -
190 - test_cmp expected observed
183 +test_expect_success 'verify sparse:path=pattern1 fails' '
184 + test_must_fail git -C r3 rev-list --quiet --objects \
185 + --filter-print-omitted --filter=sparse:path=../pattern1 HEAD
186 '
187
193 -test_expect_success 'verify sparse:path=pattern2 omits both sparse2 files' '
194 - git -C r3 ls-files -s sparse2 dir1/sparse2 >ls_files_result &&
195 - awk -f print_2.awk ls_files_result |
196 - sort >expected &&
197 -
198 - git -C r3 rev-list --quiet --objects --filter-print-omitted \
199 - --filter=sparse:path=../pattern2 HEAD >revs &&
200 - awk -f print_1.awk revs |
201 - sed "s/~//" |
202 - sort >observed &&
203 -
204 - test_cmp expected observed
188 +test_expect_success 'verify sparse:path=pattern2 fails' '
189 + test_must_fail git -C r3 rev-list --quiet --objects \
190 + --filter-print-omitted --filter=sparse:path=../pattern2 HEAD
191 '
192
193 # Test sparse:oid=<oid-ish> filter.
208 -# Like sparse:path, but we get the sparse-checkout specification from
209 -# a blob rather than a file on disk.
194 +# Use a blob containing a sparse-checkout specification to filter
195 +# out blobs not required for the corresponding sparse-checkout. We do not
196 +# require sparse-checkout to actually be enabled.
197
198 test_expect_success 'setup r3 part 2' '
199 echo dir1/ >r3/pattern &&