[12/20] commit-graph.c: prevent overflow in add_graph_to_chain()

Commit Message

Taylor Blau July 12, 2023, 11:37 p.m. UTC

The commit-graph uses a fanout table with 4-byte entries to store the
number of commits at each shard of the commit-graph. So it is OK to have
a commit graph with as many as 2^32-1 stored commits. But we risk
overflowing any computation which may exceed the 32-bit (unsigned)
maximum when those computations are (incorrectly) performed using 32-bit
operands.

There are a couple of spots in `add_graph_to_chain()` where we could
potentially overflow the result:

  - First, when comparing the list of existing entries in the
    commit-graph chain. It is unlikely that this should ever overflow,
    since it would require having roughly 2^32-1/g->hash_len
    commit-graphs in the chain. But let's guard that computation with a
    `st_mult()` just to be safe.

  - Second, when computing the number of commits in the graph added to
    the front of the chain. This value is also a 32-bit unsigned, but we
    should make sure that it does not grow beyond the maximum value.

Signed-off-by: Taylor Blau <[e-mail]>
---
 commit-graph.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Patch

diff --git a/commit-graph.c b/commit-graph.c
index 538f96b27a..99af73e40a 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -481,7 +481,7 @@  static int add_graph_to_chain(struct commit_graph *g,
 
 		if (!cur_g ||
 		    !oideq(&oids[n], &cur_g->oid) ||
-		    !hasheq(oids[n].hash, g->chunk_base_graphs + g->hash_len * n)) {
+		    !hasheq(oids[n].hash, g->chunk_base_graphs + st_mult(g->hash_len, n))) {
 			warning(_("commit-graph chain does not match"));
 			return 0;
 		}
@@ -491,8 +491,15 @@  static int add_graph_to_chain(struct commit_graph *g,
 
 	g->base_graph = chain;
 
-	if (chain)
+	if (chain) {
+		if (unsigned_add_overflows(chain->num_commits,
+					   chain->num_commits_in_base)) {
+			warning(_("commit count in base graph too high: %"PRIuMAX),
+				(uintmax_t)chain->num_commits_in_base);
+			return 0;
+		}
 		g->num_commits_in_base = chain->num_commits + chain->num_commits_in_base;
+	}
 
 	return 1;
 }