commit-graph: simplify parse_commit_graph() #1

While we iterate over all entries of the Chunk Lookup table we make
sure that we don't attempt to read past the end of the mmap-ed
commit-graph file, and check in each iteration that the chunk ID and
offset we are about to read is still within the mmap-ed memory region.
However, these checks in each iteration are not really necessary,
because the number of chunks in the commit-graph file is already known
before this loop from the just parsed commit-graph header.

So let's check that the commit-graph file is large enough for all
entries in the Chunk Lookup table before we start iterating over those
entries, and drop those per-iteration checks.  While at it, take into
account the size of everything that is necessary to have a valid
commit-graph file, i.e. the size of the header, the size of the
mandatory OID Fanout chunk, and the size of the signature in the
trailer as well.

Note that this necessitates the change of the error message as well,
and, consequently, have to update the 'detect incorrect chunk count'
test in 't5318-commit-graph.sh' as well.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/commit-graph.c b/commit-graph.c
index 6ed649388d6614b49142187e0f999c023f83e693..9927762f18cfbaa78c7692a8a5521ba2f57ddfb0 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -272,6 +272,15 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
        graph->data = graph_map;
        graph->data_len = graph_size;
 
+       if (graph_size < GRAPH_HEADER_SIZE +
+                        (graph->num_chunks + 1) * GRAPH_CHUNKLOOKUP_WIDTH +
+                        GRAPH_FANOUT_SIZE + the_hash_algo->rawsz) {
+               error(_("commit-graph file is too small to hold %u chunks"),
+                     graph->num_chunks);
+               free(graph);
+               return NULL;
+       }
+
        last_chunk_id = 0;
        last_chunk_offset = 8;
        chunk_lookup = data + 8;
@@ -280,13 +289,6 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
                uint64_t chunk_offset;
                int chunk_repeated = 0;
 
-               if (data + graph_size - chunk_lookup <
-                   GRAPH_CHUNKLOOKUP_WIDTH) {
-                       error(_("commit-graph chunk lookup table entry missing; file may be incomplete"));
-                       free(graph);
-                       return NULL;
-               }
-
                chunk_id = get_be32(chunk_lookup + 0);
                chunk_offset = get_be64(chunk_lookup + 4);
 

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 79e7fbcd40e2d44e34da87fe822f5173dfc9af5d..1073f9e3cf2c31c5ca33580d7c94bfe59ea73415 100755 (executable)
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -574,7 +574,8 @@ test_expect_success 'detect invalid checksum hash' '
 
 test_expect_success 'detect incorrect chunk count' '
        corrupt_graph_and_verify $GRAPH_BYTE_CHUNK_COUNT "\377" \
-               "chunk lookup table entry missing" $GRAPH_CHUNK_LOOKUP_OFFSET
+               "commit-graph file is too small to hold [0-9]* chunks" \
+               $GRAPH_CHUNK_LOOKUP_OFFSET
 '
 
 test_expect_success 'git fsck (checks commit-graph)' '