undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

pack-bitmap.c: check reads more aggressively when loading

Browse Source 
Before 'load_bitmap_entries_v1()' reads an actual EWAH bitmap, it should
check that it can safely do so by ensuring that there are at least 6
bytes available to be read (four for the commit's index position, and
then two more for the xor offset and flags, respectively).

Likewise, it should check that the commit index it read refers to a
legitimate object in the pack.

The first fix catches a truncation bug that was exposed when testing,
and the second is purely precautionary.

There are some possible future improvements, not pursued here. They are:

  - Computing the correct boundary of the bitmap itself in the caller
    and ensuring that we don't read past it. This may or may not be
    worth it, since in a truncation situation, all bets are off: (is the
    trailer still there and the bitmap entries malformed, or is the
    trailer truncated?). The best we can do is try to read what's there
    as if it's correct data (and protect ourselves when it's obviously
    bogus).

  - Avoid the magic "6" by teaching read_be32() and read_u8() (both of
    which are custom helpers for this function) to check sizes before
    advancing the pointers.

  - Adding more tests in this area. Testing these truncation situations
    are remarkably fragile to even subtle changes in the bitmap
    generation. So, the resulting tests are likely to be quite brittle.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Taylor Blau 2020-12-08 17:04:26 -05:00 committed by Junio C Hamano
parent 928e3f42ad
commit c6b0c3910c
1 changed files with 6 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
pack-bitmap.c
View File

@ -229,11 +229,16 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
uint32_t commit_idx_pos; uint32_t commit_idx_pos;
struct object_id oid; struct object_id oid;
if (index->map_size - index->map_pos < 6)
return error("corrupt ewah bitmap: truncated header for entry %d", i);
commit_idx_pos = read_be32(index->map, &index->map_pos); commit_idx_pos = read_be32(index->map, &index->map_pos);
xor_offset = read_u8(index->map, &index->map_pos); xor_offset = read_u8(index->map, &index->map_pos);
flags = read_u8(index->map, &index->map_pos); flags = read_u8(index->map, &index->map_pos);
nth_packed_object_id(&oid, index->pack, commit_idx_pos); if (nth_packed_object_id(&oid, index->pack, commit_idx_pos) < 0)
return error("corrupt ewah bitmap: commit index %u out of range",
(unsigned)commit_idx_pos);
bitmap = read_bitmap_1(index); bitmap = read_bitmap_1(index);
if (!bitmap) if (!bitmap)