summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLibravatar SZEDER Gábor <szeder.dev@gmail.com>2020-06-05 13:00:29 +0000
committerLibravatar Junio C Hamano <gitster@pobox.com>2020-06-08 12:28:49 -0700
commit2ad4f1a7c43cac4ba52cff56f9090d3322c67224 (patch)
treedc6639ba39c54bf64a3eff22faea35cdc06a9870
parentcommit-graph: clean up #includes (diff)
downloadtgif-2ad4f1a7c43cac4ba52cff56f9090d3322c67224.tar.xz
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>
-rw-r--r--commit-graph.c16
-rwxr-xr-xt/t5318-commit-graph.sh3
2 files changed, 11 insertions, 8 deletions
diff --git a/commit-graph.c b/commit-graph.c
index 6ed649388d..9927762f18 100644
--- 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 79e7fbcd40..1073f9e3cf 100755
--- 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)' '