git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 10/13] packfile: validate pack index v3 tables

From
Christian Couder <christian.couder@gmail.com>
Date
Oct 23, 2026, 07:25 UTC
Message-ID
<20261023072559.3204273-11-christian.couder@gmail.com>
In-Reply-To
<20261023072559.3204273-1-christian.couder@gmail.com>

A previous commit implemented loading a pack index v3, but didn't check much of its format when doing so.

To avoid reading outside a v3 index, if it's corrupt or crafted, let's check that its header, its format entries and all its tables lie within the file, when loading it.

Let's also reject duplicate formats and short names longer than the full object names.

The values in the order maps are checked when they are read, rather than when loading the index, so that loading stays cheap. A new test in t5313 ensures that an invalid order map entry is detected.

In the pack revindex, let's also check that the 8-byte offsets we read lie within the index.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 hash-lookup.c                 |  5 ++-
 pack-revindex.c               |  5 ++-
 packfile.c                    | 71 ++++++++++++++++++++++++++---------
 t/t5313-pack-bounds-checks.sh | 17 +++++++++
 4 files changed, 78 insertions(+), 20 deletions(-)
diff --git a/hash-lookup.c b/hash-lookup.c
index a77ccad8b5..3f0725f7ec 100644
--- a/hash-lookup.c
+++ b/hash-lookup.c
@@ -144,8 +144,9 @@ int bsearch_hash_v3(const unsigned char *hash, const unsigned char *full_oid,
 		    const uint32_t *pack_map, const unsigned char *short_oid,
 		    size_t stride, size_t hashsz, uint32_t *result)
 {
+	uint32_t nr = (full_oid - short_oid) / stride;
 	uint32_t obj_off, index;
-	int res = bsearch_hash_1(hash, short_oid, stride, stride, (full_oid - short_oid) / stride,
+	int res = bsearch_hash_1(hash, short_oid, stride, stride, nr,
 				 0, &obj_off);
 	if (result)
 		*result = obj_off;
@@ -157,6 +158,8 @@ int bsearch_hash_v3(const unsigned char *hash, const unsigned char *full_oid,
 	 * to find the full object ID.  index is the offset in pack order.
 	 */
 	index = ntohl(pack_map[obj_off]);
+	if (index >= nr)
+		return 0;
 	if (!memcmp(hash, full_oid + (hashsz * index), hashsz))
 		return 1;
 	return 0;
diff --git a/pack-revindex.c b/pack-revindex.c
index fb81db8033..4e6923cfd3 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -150,7 +150,10 @@ static void create_pack_revindex(struct packed_git *p)
 			if (!(off & 0x80000000)) {
 				p->revindex[i].offset = off;
 			} else {
-				p->revindex[i].offset = get_be64(off_64 + st_mult(lowoff, 2));
+				const uint32_t *ptr = off_64 + st_mult(lowoff, 2);
+
+				check_pack_index_ptr(p, ptr);
+				p->revindex[i].offset = get_be64(ptr);
 			}
 			p->revindex[i].nr = i;
 		}
diff --git a/packfile.c b/packfile.c
index 95207c0e19..f6e2db45d5 100644
--- a/packfile.c
+++ b/packfile.c
@@ -180,50 +180,81 @@ int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
 			return error("pack too large for current definition of off_t in %s", path);
 		p->crc_offset = st_add(8 + 4 * 256, st_mult(nr, hashsz));
 	} else if (version == 3) {
-		uint32_t hdrlen, num_formats, trailer_offset, main_algo;
+		uint32_t hdrlen, num_formats, main_algo;
+		uint64_t trailer_offset;
 		const uint32_t *q = index;
-		int i;
 
+		/* Fixed part of the header (after signature and version) */
 		hdrlen = ntohl(*q++);
-		if (idx_size < hdrlen)
-			return error("v3 index has truncated header");
-
 		nr = ntohl(*q++);
 		num_formats = ntohl(*q++);
 		if (num_formats < 1 || num_formats > ARRAY_SIZE(p->formats))
-			return error("too many formats for v3 index");
+			return error("v3 index %s has an invalid number of formats: %"PRIu32,
+				     path, num_formats);
+
+		/* Then 16 bytes per format and the 8-byte offset of the trailer */
+		if (hdrlen < 20 + num_formats * (4 + 4 + 8) + 8 ||
+		    hdrlen > idx_size - 2 * hashsz)
+			return error("v3 index %s has an invalid header length", path);
+
+		trailer_offset = get_be64(q + num_formats * 4);
+		if (trailer_offset != idx_size - 2 * hashsz)
+			return error("v3 index %s has a bad size", path);
+
 		memset(p->formats, 0, sizeof(p->formats));
 		for (i = 0; i < num_formats; i++) {
 			uint32_t format_id = ntohl(*q++);
 			uint32_t format = hash_algo_by_id(format_id);
 			struct packed_git_format *pfp;
+			uint64_t end;
 
 			if (format == GIT_HASH_UNKNOWN)
-				return error("v3 index with unknown format_id 0x%08x", format_id);
+				return error("v3 index %s has an unknown format_id 0x%08"PRIx32,
+					     path, format_id);
 			/*
 			 * We store at the value of the constant minus 1 to
 			 * avoid having to store a useless entry at offset 0.
 			 */
 			pfp = &p->formats[format-1];
+			if (pfp->data_offset)
+				return error("v3 index %s has duplicate format_id 0x%08"PRIx32,
+					     path, format_id);
 
 			pfp->short_name_len = ntohl(*q++);
 			pfp->data_offset = get_be64(q);
 			q += 2;
 
+			if (!pfp->short_name_len ||
+			    pfp->short_name_len > hash_algos[format].rawsz)
+				return error("v3 index %s has an invalid short name length",
+					     path);
+
+			if (pfp->data_offset < hdrlen ||
+			    pfp->data_offset > trailer_offset)
+				return error("v3 index %s has an invalid table offset", path);
+
+			/* No overflow as pfp->data_offset < idx_size and nr < 2^32 */
 			pfp->full_oid_offset = pfp->data_offset +
-					       (pfp->short_name_len * nr);
+					       (uint64_t)pfp->short_name_len * nr;
 			pfp->order_map_offset = pfp->full_oid_offset +
-						(hash_algos[format].rawsz * nr);
-			if (!i)
-				p->crc_offset = pfp->order_map_offset + nr * 4;
+						(uint64_t)hash_algos[format].rawsz * nr;
+			end = pfp->order_map_offset + (uint64_t)nr * 4;
+
+			/* After the first format tables: CRC32 and 4-byte offset tables */
+			if (!i) {
+				p->crc_offset = end;
+				end += (uint64_t)nr * (4 + 4); /* CRC32 + offset */
+			}
+
+			if (pfp->order_map_offset % 4 || end > trailer_offset)
+				return error("v3 index %s has invalid tables", path);
 		}
+
 		main_algo = hash_algo_by_length(hashsz);
 		if (main_algo == GIT_HASH_UNKNOWN ||
 		    !p->formats[main_algo - 1].data_offset)
-			return error("v3 index with missing data for this repository's algorithm");
-		trailer_offset = get_be64(q);
-		if (trailer_offset + 2 * hashsz != idx_size)
-			return error("v3 index with bad size\n");
+			return error("v3 index %s is missing data for the main hash algorithm",
+				     path);
 	}
 
 	p->index_version = version;
@@ -1897,10 +1928,14 @@ uint32_t nth_packed_object_pack_order_algop(const struct packed_git *p, uint32_t
 	const unsigned char *index = p->index_data;
 	const unsigned int algo = hash_algo_by_ptr(algop);
 	const struct packed_git_format *pfp = &p->formats[algo-1];
+	uint32_t pos = ntohl(*(uint32_t *)(index +
+					   st_add(pfp->order_map_offset,
+						  st_mult(4, n))));
 
-	return ntohl(*(uint32_t *)(index +
-				   st_add(pfp->order_map_offset,
-					  st_mult(4, n))));
+	if (pos >= p->num_objects)
+		die(_("invalid object position in pack index for %s (corrupt index?)"),
+		    p->pack_name);
+	return pos;
 }
 
 off_t find_pack_entry_one(const struct object_id *oid,
diff --git a/t/t5313-pack-bounds-checks.sh b/t/t5313-pack-bounds-checks.sh
index 5be01260d7..ab2b0bb9c6 100755
--- a/t/t5313-pack-bounds-checks.sh
+++ b/t/t5313-pack-bounds-checks.sh
@@ -45,6 +45,14 @@ extended_table () {
 	echo $(($(ofs_table "$1") + 4*$1))
 }
 
+# Offset in a single-format v3 .idx with no extended offsets to the
+# start of its order map. $1 is its number of objects, nr.
+# The order map (4*nr) is followed by the CRC32 table (4*nr), the
+# 4-byte offset table (4*nr) and by the trailer (2*rawsz).
+v3_order_map () {
+	echo $(($(wc -c <"$idx") - 2*$(test_oid rawsz) - 12*$1))
+}
+
 test_expect_success 'setup' '
 	test_oid_cache <<-EOF
 	oid000 sha1:1485
@@ -171,6 +179,15 @@ test_expect_success 'bogus offset inside v2 extended table' '
 	test_must_fail git index-pack --verify $pack
 '
 
+test_expect_success 'bogus order map entry (v3)' '
+	do_pack $object --index-version=3 &&
+	munge $idx $(v3_order_map 1) "\377\377\377\377" &&
+	clear_base &&
+	test_must_fail git cat-file blob $object &&
+	test_must_fail git cat-file --batch-all-objects --batch-check 2>err &&
+	test_grep "corrupt index" err
+'
+
 test_expect_success 'bogus OFS_DELTA in packfile' '
 	# Generate a pack with a delta in it.
 	base=$(test-tool genrandom foo 3000 | git hash-object --stdin -w) &&

-- 
2.56.0.215.g47fdf2208c
Previous: Christian CouderNext: Christian Couder
Message 11 of 17 in “SHA-1/SHA-256 interoperability, part 3: pack index v3”
  1. 00/13 SHA-1/SHA-256 interoperability, part 3: pack index v3Christian Couder, Oct 23, 2026
  2. 01/13 loose: allow looking up in object map with sourceChristian Couder, Oct 23, 2026
  3. 02/13 loose: rename repo_add_loose_object_mapChristian Couder, Oct 23, 2026
  4. 03/13 loose: add a flag to control whether we write mapped OIDsChristian Couder, Oct 23, 2026
  5. 04/13 odb: implement interoperability functionsChristian Couder, Oct 23, 2026
  6. 05/13 object-file-convert: implement mapping in terms of object DBChristian Couder, Oct 23, 2026
  7. 06/13 pack-write: add support for pack index v3Christian Couder, Oct 23, 2026
  8. 07/13 builtin: populate pack index v3 fields when indexingChristian Couder, Oct 23, 2026
  9. 08/13 object-file: handle compatibility algorithmChristian Couder, Oct 23, 2026
  10. 09/13 packfile: add support for reading pack index v3Christian Couder, Oct 23, 2026
  11. 10/13 packfile: validate pack index v3 tablesChristian Couder, Oct 23, 2026
  12. 11/13 loose: map object IDs using pack index v3Christian Couder, Oct 23, 2026
  13. 12/13 packfile: allow looking up compat object IDs in packsChristian Couder, Oct 23, 2026
  14. 13/13 doc: move pack index v3 documentationChristian Couder, Oct 23, 2026
  15. Ben KnobleOct 23, 2026
  16. Junio C HamanoOct 23, 2026
  17. Christian CouderOct 27, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.