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

[PATCH 09/20] midx: check size of object offset chunk

From
Jeff King <peff@peff.net>
Date
Oct 9, 2023, 21:05 UTC
Message-ID
<20231009210527.GI3282181@coredump.intra.peff.net>
In-Reply-To
<20231009205544.GA3281950@coredump.intra.peff.net>

The object offset chunk has one fixed-size entry for each object in the midx. But since we don't check its size, we may access out-of-bounds memory if we see a corrupt or malicious midx file.

Sine the entries are fixed-size, the total length can be known up-front, and we can just check it while parsing the chunk (this is similar to what we do when opening pack idx files, which contain a similar offset table).

Signed-off-by: Jeff King <peff@peff.net>
---
 midx.c                      | 15 ++++++++++++++-
 t/t5319-multi-pack-index.sh | 10 ++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/midx.c b/midx.c
index 98f84fbe61..7b1b45f381 100644
--- a/midx.c
+++ b/midx.c
@@ -88,6 +88,19 @@ static int midx_read_oid_lookup(const unsigned char *chunk_start,
 	return 0;
 }
 
+static int midx_read_object_offsets(const unsigned char *chunk_start,
+				    size_t chunk_size, void *data)
+{
+	struct multi_pack_index *m = data;
+	m->chunk_object_offsets = chunk_start;
+
+	if (chunk_size != st_mult(m->num_objects, MIDX_CHUNK_OFFSET_WIDTH)) {
+		error(_("multi-pack-index object offset chunk is the wrong size"));
+		return 1;
+	}
+	return 0;
+}
+
 struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local)
 {
 	struct multi_pack_index *m = NULL;
@@ -164,7 +177,7 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
 		die(_("multi-pack-index required OID fanout chunk missing or corrupted"));
 	if (read_chunk(cf, MIDX_CHUNKID_OIDLOOKUP, midx_read_oid_lookup, m))
 		die(_("multi-pack-index required OID lookup chunk missing or corrupted"));
-	if (pair_chunk_unsafe(cf, MIDX_CHUNKID_OBJECTOFFSETS, &m->chunk_object_offsets))
+	if (read_chunk(cf, MIDX_CHUNKID_OBJECTOFFSETS, midx_read_object_offsets, m))
 		die(_("multi-pack-index required object offsets chunk missing or corrupted"));
 
 	pair_chunk_unsafe(cf, MIDX_CHUNKID_LARGEOFFSETS, &m->chunk_large_offsets);
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 34f5944142..30687d5452 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -1108,4 +1108,14 @@ test_expect_success 'reader handles unaligned chunks' '
 	test_cmp expect.err err
 '
 
+test_expect_success 'reader notices too-small object offset chunk' '
+	corrupt_chunk OOFF clear 00000000 &&
+	test_must_fail git log 2>err &&
+	cat >expect <<-\EOF &&
+	error: multi-pack-index object offset chunk is the wrong size
+	fatal: multi-pack-index required object offsets chunk missing or corrupted
+	EOF
+	test_cmp expect err
+'
+
 test_done
-- 
2.42.0.884.g35e1fe1a6a
Previous: Jeff KingNext: Taylor Blau
Message 24 of 67 in “bounds-checks for chunk-based files”
  1. 0/20 bounds-checks for chunk-based filesJeff King, Oct 9, 2023
  2. 01/20 chunk-format: note that pair_chunk() is unsafeJeff King, Oct 9, 2023
  3. Taylor BlauOct 10, 2023
  4. Jeff KingOct 11, 2023
  5. 02/20 t: add library for munging chunk-format filesJeff King, Oct 9, 2023
  6. Taylor BlauOct 10, 2023
  7. 03/20 midx: stop ignoring malformed oid fanout chunkJeff King, Oct 9, 2023
  8. Taylor BlauOct 10, 2023
  9. Jeff KingOct 11, 2023
  10. 04/20 commit-graph: check size of oid fanout chunkJeff King, Oct 9, 2023
  11. Taylor BlauOct 11, 2023
  12. Taylor BlauOct 11, 2023
  13. Jeff KingOct 11, 2023
  14. 05/20 midx: check size of oid lookup chunkJeff King, Oct 9, 2023
  15. 06/20 commit-graph: check consistency of fanout tableJeff King, Oct 9, 2023
  16. Taylor BlauOct 11, 2023
  17. 07/20 midx: check size of pack names chunkJeff King, Oct 9, 2023
  18. Taylor BlauOct 11, 2023
  19. Jeff KingOct 11, 2023
  20. 08/20 midx: enforce chunk alignment on readingJeff King, Oct 9, 2023
  21. Taylor BlauOct 11, 2023
  22. Taylor BlauOct 11, 2023
  23. Jeff KingOct 11, 2023
  24. 09/20 midx: check size of object offset chunkJeff King, Oct 9, 2023
  25. Taylor BlauOct 11, 2023
  26. 10/20 midx: bounds-check large offset chunkJeff King, Oct 9, 2023
  27. Taylor BlauOct 11, 2023
  28. Jeff KingOct 11, 2023
  29. 11/20 midx: check size of revindex chunkJeff King, Oct 9, 2023
  30. Taylor BlauOct 11, 2023
  31. 12/20 commit-graph: check size of commit data chunkJeff King, Oct 9, 2023
  32. Taylor BlauOct 11, 2023
  33. Jeff KingOct 11, 2023
  34. 13/20 commit-graph: detect out-of-bounds extra-edges pointersJeff King, Oct 9, 2023
  35. Taylor BlauOct 11, 2023
  36. 14/20 commit-graph: bounds-check base graphs chunkJeff King, Oct 9, 2023
  37. Taylor BlauOct 11, 2023
  38. 15/20 commit-graph: check size of generations chunkJeff King, Oct 9, 2023
  39. 16/20 commit-graph: bounds-check generation overflow chunkJeff King, Oct 9, 2023
  40. 17/20 commit-graph: check bounds when accessing BDAT chunkJeff King, Oct 9, 2023
  41. Taylor BlauOct 11, 2023
  42. Jeff KingOct 11, 2023
  43. 18/20 commit-graph: check bounds when accessing BIDX chunkJeff King, Oct 9, 2023
  44. Taylor BlauOct 11, 2023
  45. 19/20 commit-graph: detect out-of-order BIDX offsetsJeff King, Oct 9, 2023
  46. Taylor BlauOct 11, 2023
  47. 20/20 chunk-format: drop pair_chunk_unsafe()Jeff King, Oct 9, 2023
  48. Taylor BlauOct 11, 2023
  49. Jeff KingOct 11, 2023
  50. 0/8 chunk-format: introduce `pair_chunk_expect()` convenience APITaylor Blau, Oct 13, 2023
  51. 1/8 chunk-format: introduce `pair_chunk_expect()` helperTaylor Blau, Oct 13, 2023
  52. 2/8 commit-graph: read `OIDF` chunk with `pair_chunk_expect()`Taylor Blau, Oct 13, 2023
  53. 3/8 commit-graph: read `CDAT` chunk with `pair_chunk_expect()`Taylor Blau, Oct 13, 2023
  54. 4/8 commit-graph: read `GDAT` chunk with `pair_chunk_expect()`Taylor Blau, Oct 13, 2023
  55. 5/8 commit-graph: read `BIDX` chunk with `pair_chunk_expect()`Taylor Blau, Oct 13, 2023
  56. Taylor BlauOct 13, 2023
  57. Junio C HamanoOct 14, 2023
  58. Jeff KingOct 20, 2023
  59. 6/8 midx: read `OIDF` chunk with `pair_chunk_expect()`Taylor Blau, Oct 13, 2023
  60. Junio C HamanoOct 13, 2023
  61. 7/8 midx: read `OIDL` chunk with `pair_chunk_expect()`Taylor Blau, Oct 13, 2023
  62. 8/8 midx: read `OOFF` chunk with `pair_chunk_expect()`Taylor Blau, Oct 13, 2023
  63. Jeff KingOct 20, 2023
  64. 21/20 t5319: make corrupted large-offset test more robustJeff King, Oct 14, 2023
  65. Junio C HamanoOct 14, 2023
  66. Jeff KingOct 15, 2023
  67. Junio C HamanoOct 15, 2023

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.