upload-pack.c: don't free allowed_filters util pointers

To keep track of which object filters are allowed or not, 'git
upload-pack' stores the name of each filter in a string_list, and sets
it ->util pointer to be either 0 or 1, indicating whether it is banned
or allowed.

Later on, we attempt to clear that list, but we incorrectly ask for the
util pointers to be free()'d, too. This behavior (introduced back in
6dd3456a8c (upload-pack.c: allow banning certain object filter(s),
2020-08-03)) leads to an invalid free, and causes us to crash.

In order to trigger this, one needs to fetch from a server that (a) has
at least one object filter allowed, and (b) issue a fetch that contains
a subset of the allowed filters (i.e., we cannot ask for a banned
filter, since this causes us to die() before we hit the bogus
string_list_clear()).

In that case, whatever banned filters exist will cause a noop free()
(since those ->util pointers are set to 0), but the first allowed filter
we try to free will crash us.

We never noticed this in the tests because we didn't have an example of
setting 'uploadPackFilter' configuration variables and then following up
with a valid fetch. The first new 'git clone' prevents further
regression here. For good measure on top, add a test which checks the
same behavior at a tree depth greater than 0.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index f4d49d833577c33f741adfb8e5ded0cdae2a4a8b..5da945cf159e532a1cec7899bf1b0d658567c6b0 100755 (executable)
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -281,7 +281,15 @@ test_expect_success 'upload-pack limits tree depth filters' '
        test_config -C srv.bare uploadpackfilter.tree.maxDepth 0 &&
        test_must_fail ok=sigpipe git clone --no-checkout --filter=tree:1 \
                "file://$(pwd)/srv.bare" pc3 2>err &&
-       test_i18ngrep "tree filter allows max depth 0, but got 1" err
+       test_i18ngrep "tree filter allows max depth 0, but got 1" err &&
+
+       git clone --no-checkout --filter=tree:0 "file://$(pwd)/srv.bare" pc4 &&
+
+       test_config -C srv.bare uploadpackfilter.tree.maxDepth 5 &&
+       git clone --no-checkout --filter=tree:5 "file://$(pwd)/srv.bare" pc5 &&
+       test_must_fail ok=sigpipe git clone --no-checkout --filter=tree:6 \
+               "file://$(pwd)/srv.bare" pc6 2>err &&
+       test_i18ngrep "tree filter allows max depth 5, but got 6" err
 '
 
 test_expect_success 'partial clone fetches blobs pointed to by refs even if normally filtered out' '

diff --git a/upload-pack.c b/upload-pack.c
index 1006bebd502b884f979ce7054153a654bbbb208f..be8fffbc071ca59820e6255283c1c0dc9c3c6f6c 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -154,7 +154,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)
        string_list_clear(&data->deepen_not, 0);
        object_array_clear(&data->extra_edge_obj);
        list_objects_filter_release(&data->filter_options);
-       string_list_clear(&data->allowed_filters, 1);
+       string_list_clear(&data->allowed_filters, 0);
 
        free((char *)data->pack_objects_hook);
 }