[PATCH 2/3] walker_fetch(): avoid raw array length computation

From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 2/3] walker_fetch(): avoid raw array length computation
Date: Thu, 30 Jan 2020 04:52:32 -0500	[thread overview]
Message-ID: <20200130095232.GB840531@coredump.intra.peff.net> (raw)
In-Reply-To: <20200130095155.GA839988@coredump.intra.peff.net>

We compute the length of an array of object_id's with a raw
multiplication. In theory this could trigger an integer overflow which
would cause an under-allocation (and eventually an out of bounds write).

I doubt this can be triggered in practice, since you'd need to feed it
an enormous number of target objects, which would typically come from
the ref advertisement and be using proportional memory. And even on
64-bit systems, where "int" is much smaller than "size_t", that should
hold: even though "targets" is an int, the multiplication will be done
as a size_t because of the use of sizeof().

But we can easily fix it by using ALLOC_ARRAY(), which uses st_mult()
under the hood.

Signed-off-by: Jeff King <peff@peff.net>
---
 walker.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/walker.c b/walker.c
index 06cd2bd569..bb010f7a2b 100644
--- a/walker.c
+++ b/walker.c
@@ -261,12 +261,14 @@ int walker_fetch(struct walker *walker, int targets, char **target,
 	struct strbuf refname = STRBUF_INIT;
 	struct strbuf err = STRBUF_INIT;
 	struct ref_transaction *transaction = NULL;
-	struct object_id *oids = xmalloc(targets * sizeof(struct object_id));
+	struct object_id *oids;
 	char *msg = NULL;
 	int i, ret = -1;
 
 	save_commit_buffer = 0;
 
+	ALLOC_ARRAY(oids, targets);
+
 	if (write_ref) {
 		transaction = ref_transaction_begin(&err);
 		if (!transaction) {
-- 
2.25.0.515.gaba5347bc6

