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

Commit Message

Taylor Blau June 14, 2024, 7:23 p.m. UTC

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 (adding 24 to the left-hand side of our
inequality to account for 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 <[e-mail]>
Signed-off-by: Taylor Blau <[e-mail]>
---
 pack-bitmap.c | 5 +++++
 1 file changed, 5 insertions(+)

Patch

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 70230e2647..ae13d7ee3b 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -238,6 +238,11 @@  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_add(st_mult(index->pseudo_merges.nr,
+						   sizeof(uint64_t)),
+					   24) > table_size)
+					return error(_("corrupted bitmap index file, pseudo-merge table too short"));
+
 				CALLOC_ARRAY(index->pseudo_merges.v,
 					     index->pseudo_merges.nr);