Skip to content

Commit c6b0c39

Browse files
ttaylorrgitster
authored andcommitted
pack-bitmap.c: check reads more aggressively when loading
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 <[email protected]> Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 928e3f4 commit c6b0c39

File tree

1 file changed

+6
-1
lines changed

1 file changed

+6
-1
lines changed

pack-bitmap.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,11 +229,16 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
229229
uint32_t commit_idx_pos;
230230
struct object_id oid;
231231

232+
if (index->map_size - index->map_pos < 6)
233+
return error("corrupt ewah bitmap: truncated header for entry %d", i);
234+
232235
commit_idx_pos = read_be32(index->map, &index->map_pos);
233236
xor_offset = read_u8(index->map, &index->map_pos);
234237
flags = read_u8(index->map, &index->map_pos);
235238

236-
nth_packed_object_id(&oid, index->pack, commit_idx_pos);
239+
if (nth_packed_object_id(&oid, index->pack, commit_idx_pos) < 0)
240+
return error("corrupt ewah bitmap: commit index %u out of range",
241+
(unsigned)commit_idx_pos);
237242

238243
bitmap = read_bitmap_1(index);
239244
if (!bitmap)

0 commit comments

Comments
 (0)