[PATCH v2 2/2] pack-bitmap.c: ensure pseudo-merge offset reads are bounded
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Jun 14, 2024, 19:23 UTC
- Message-ID
- <8abd564e7cbea8a3a165db86354d10a3f966d50f.1718392943.git.me@ttaylorr.com>
- In-Reply-To
- <cover.1718392943.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 (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 <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> --- pack-bitmap.c | 5 +++++ 1 file changed, 5 insertions(+)
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);
-- 2.45.0.33.g0a16399d14.dirty