commit-graph: check size of commit data chunk

We expect a commit-graph file to have a fixed-size data record for each commit in the file (and we know the number of commits to expct from the size of the lookup table). If we encounter a file where this is too small, we'll look past the end of the chunk (and possibly even off the mapped memory). We can fix this by checking the size up front when we record the pointer. The included test doesn't segfault, since it ends up reading bytes from another chunk. But it produces nonsense results, since the values it reads are garbage. Our test notices this by comparing the output to a non-corrupted run of the same command (and of course we also check that the expected error is printed to stderr). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 9, 2023 at 17:05 UTC b72df612afc12b46ea003732d739d7d746871773
2 files changed +20 -1
commit-graph.c
+11 -1
@@ -340,6 +340,16 @@ static int graph_read_oid_lookup(const unsigned char *chunk_start,
340 return 0;
341 }
342
343 +static int graph_read_commit_data(const unsigned char *chunk_start,
344 + size_t chunk_size, void *data)
345 +{
346 + struct commit_graph *g = data;
347 + if (chunk_size != g->num_commits * GRAPH_DATA_WIDTH)
348 + return error("commit-graph commit data chunk is wrong size");
349 + g->chunk_commit_data = chunk_start;
350 + return 0;
351 +}
352 +
353 static int graph_read_bloom_data(const unsigned char *chunk_start,
354 size_t chunk_size, void *data)
355 {
@@ -422,7 +432,7 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
432
433 read_chunk(cf, GRAPH_CHUNKID_OIDFANOUT, graph_read_oid_fanout, graph);
434 read_chunk(cf, GRAPH_CHUNKID_OIDLOOKUP, graph_read_oid_lookup, graph);
425 - pair_chunk_unsafe(cf, GRAPH_CHUNKID_DATA, &graph->chunk_commit_data);
435 + read_chunk(cf, GRAPH_CHUNKID_DATA, graph_read_commit_data, graph);
436 pair_chunk_unsafe(cf, GRAPH_CHUNKID_EXTRAEDGES, &graph->chunk_extra_edges);
437 pair_chunk_unsafe(cf, GRAPH_CHUNKID_BASE, &graph->chunk_base_graphs);
438
t/t5318-commit-graph.sh
+9
@@ -870,4 +870,13 @@ test_expect_success 'reader notices out-of-bounds fanout' '
870 test_cmp expect.err err
871 '
872
873 +test_expect_success 'reader notices too-small commit data chunk' '
874 + check_corrupt_chunk CDAT clear 00000000 &&
875 + cat >expect.err <<-\EOF &&
876 + error: commit-graph commit data chunk is wrong size
877 + error: commit-graph is missing the Commit Data chunk
878 + EOF
879 + test_cmp expect.err err
880 +'
881 +
882 test_done