Skip to content

Commit 8d133f5

Browse files
ttaylorrgitster
authored andcommitted
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 6dd3456 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent aab179d commit 8d133f5

File tree

2 files changed

+10
-2
lines changed

2 files changed

+10
-2
lines changed

t/t5616-partial-clone.sh

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,15 @@ test_expect_success 'upload-pack limits tree depth filters' '
281281
test_config -C srv.bare uploadpackfilter.tree.maxDepth 0 &&
282282
test_must_fail ok=sigpipe git clone --no-checkout --filter=tree:1 \
283283
"file://$(pwd)/srv.bare" pc3 2>err &&
284-
test_i18ngrep "tree filter allows max depth 0, but got 1" err
284+
test_i18ngrep "tree filter allows max depth 0, but got 1" err &&
285+
286+
git clone --no-checkout --filter=tree:0 "file://$(pwd)/srv.bare" pc4 &&
287+
288+
test_config -C srv.bare uploadpackfilter.tree.maxDepth 5 &&
289+
git clone --no-checkout --filter=tree:5 "file://$(pwd)/srv.bare" pc5 &&
290+
test_must_fail ok=sigpipe git clone --no-checkout --filter=tree:6 \
291+
"file://$(pwd)/srv.bare" pc6 2>err &&
292+
test_i18ngrep "tree filter allows max depth 5, but got 6" err
285293
'
286294

287295
test_expect_success 'partial clone fetches blobs pointed to by refs even if normally filtered out' '

upload-pack.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)
154154
string_list_clear(&data->deepen_not, 0);
155155
object_array_clear(&data->extra_edge_obj);
156156
list_objects_filter_release(&data->filter_options);
157-
string_list_clear(&data->allowed_filters, 1);
157+
string_list_clear(&data->allowed_filters, 0);
158158

159159
free((char *)data->pack_objects_hook);
160160
}

0 commit comments

Comments
 (0)