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

[PATCH 2/2] pack-bitmap.c: ensure pseudo-merge offset reads are bounded

From
Taylor Blau <me@ttaylorr.com>
Date
Jun 6, 2024, 18:41 UTC
Message-ID
<0a16399d14afd527f4db63f2a4a3b0a3cbf112f1.1717699237.git.me@ttaylorr.com>
In-Reply-To
<a71ec05e5dc0c8c40e1cce14a7c5fe946437a24d.1717699237.git.me@ttaylorr.com>

After reading the pseudo-merge extension's metadata table, we allocate an array to store information about each pseudo-merge, including its byte offset within the .bitmap file itself.

This is done like so:
    pseudo_merge_ofs = index_end - 24 -
            (index->pseudo_merges.nr * sizeof(uint64_t));
    for (i = 0; i < index->pseudo_merges.nr; i++) {
            index->pseudo_merges.v[i].at = get_be64(pseudo_merge_ofs);
            pseudo_merge_ofs += sizeof(uint64_t);
    }

But if the pseudo-merge table is corrupt, we'll keep calling get_be64() past the end of the pseudo-merge extension, potentially reading off the end of the mmap'd region.

Prevent this by ensuring that we have at least `table_size - 24` many bytes available to read (subtracting 24 as the length of the metadata component).

This is sufficient to prevent us from reading off the end of the pseudo-merge extension, and ensures that all of the get_be64() calls below are in bounds.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 pack-bitmap.c | 3 +++
 1 file changed, 3 insertions(+)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 70230e2647..ad2635c025 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -238,6 +238,9 @@ static int load_bitmap_header(struct bitmap_index *index)
 				index->pseudo_merges.commits_nr = get_be32(index_end - 20);
 				index->pseudo_merges.nr = get_be32(index_end - 24);
 
+				if (st_mult(index->pseudo_merges.nr, sizeof(uint64_t)) > table_size - 24)
+					return error(_("corrupted bitmap index file, pseudo-merge table too short"));
+
 				CALLOC_ARRAY(index->pseudo_merges.v,
 					     index->pseudo_merges.nr);
 
-- 
2.45.0.32.ga71ec05e5dc.dirty
Previous: Taylor BlauNext: Junio C Hamano
Message 2 of 12 in “Documentation/technical/bitmap-format.txt: add missing position table”
  1. 1/2 Documentation/technical/bitmap-format.txt: add missing position tableTaylor Blau, Jun 6, 2024
  2. 2/2 pack-bitmap.c: ensure pseudo-merge offset reads are boundedTaylor Blau, Jun 6, 2024
  3. Junio C HamanoJun 6, 2024
  4. Taylor BlauJun 6, 2024
  5. Junio C HamanoJun 6, 2024
  6. Taylor BlauJun 6, 2024
  7. Junio C HamanoJun 14, 2024
  8. 0/2 pseudo-merge: various small fixesTaylor Blau, Jun 14, 2024
  9. 1/2 Documentation/technical/bitmap-format.txt: add missing position tableTaylor Blau, Jun 14, 2024
  10. 2/2 pack-bitmap.c: ensure pseudo-merge offset reads are boundedTaylor Blau, Jun 14, 2024
  11. Elijah NewrenJun 14, 2024
  12. Junio C HamanoJun 14, 2024

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.