commit-graph: check size of oid fanout chunk
We load the oid fanout chunk with pair_chunk(), which means we never see the size of the chunk. We just assume the on-disk file uses the appropriate size, and if it's too small we'll access random memory. It's easy to check this up-front; the fanout always consists of 256 uint32's, since it is a fanout of the first byte of the hash pointing into the oid index. These parameters can't be changed without introducing a new chunk type. This matches the similar check in the midx OIDF chunk (but note that rather than checking for the error immediately, the graph code just leaves parts of the struct NULL and checks for required fields later). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Oct 9, 2023 at 16:59 UTC
52e2e8d43dbae8c05b68efd60cde2aacf3a23890
2 files changed
+37
-2
commit-graph.c
+11
-2
@@ -305,6 +305,16 @@ static int verify_commit_graph_lite(struct commit_graph *g)
305
return 0;
306
}
307
308
+static int graph_read_oid_fanout(const unsigned char *chunk_start,
309
+ size_t chunk_size, void *data)
310
+{
311
+ struct commit_graph *g = data;
312
+ if (chunk_size != 256 * sizeof(uint32_t))
313
+ return error("commit-graph oid fanout chunk is wrong size");
314
+ g->chunk_oid_fanout = (const uint32_t *)chunk_start;
315
+ return 0;
316
+}
317
+
318
static int graph_read_oid_lookup(const unsigned char *chunk_start,
319
size_t chunk_size, void *data)
320
{
@@ -394,8 +404,7 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
404
GRAPH_HEADER_SIZE, graph->num_chunks))
405
goto free_and_return;
406
397
- pair_chunk_unsafe(cf, GRAPH_CHUNKID_OIDFANOUT,
398
- (const unsigned char **)&graph->chunk_oid_fanout);
407
+ read_chunk(cf, GRAPH_CHUNKID_OIDFANOUT, graph_read_oid_fanout, graph);
408
read_chunk(cf, GRAPH_CHUNKID_OIDLOOKUP, graph_read_oid_lookup, graph);
409
pair_chunk_unsafe(cf, GRAPH_CHUNKID_DATA, &graph->chunk_commit_data);
410
pair_chunk_unsafe(cf, GRAPH_CHUNKID_EXTRAEDGES, &graph->chunk_extra_edges);
t/t5318-commit-graph.sh
+26
@@ -2,6 +2,7 @@
2
3
test_description='commit graph'
4
. ./test-lib.sh
5
+. "$TEST_DIRECTORY"/lib-chunk.sh
6
7
GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0
8
@@ -821,4 +822,29 @@ test_expect_success 'overflow during generation version upgrade' '
822
)
823
'
824
825
+corrupt_chunk () {
826
+ graph=full/.git/objects/info/commit-graph &&
827
+ test_when_finished "rm -rf $graph" &&
828
+ git -C full commit-graph write --reachable &&
829
+ corrupt_chunk_file $graph "$@"
830
+}
831
+
832
+check_corrupt_chunk () {
833
+ corrupt_chunk "$@" &&
834
+ git -C full -c core.commitGraph=false log >expect.out &&
835
+ git -C full -c core.commitGraph=true log >out 2>err &&
836
+ test_cmp expect.out out
837
+}
838
+
839
+test_expect_success 'reader notices too-small oid fanout chunk' '
840
+ # make it big enough that the graph file is plausible,
841
+ # otherwise we hit an earlier check
842
+ check_corrupt_chunk OIDF clear $(printf "000000%02x" $(test_seq 250)) &&
843
+ cat >expect.err <<-\EOF &&
844
+ error: commit-graph oid fanout chunk is wrong size
845
+ error: commit-graph is missing the OID Fanout chunk
846
+ EOF
847
+ test_cmp expect.err err
848
+'
849
+
850
test_done