The Parallels Format Extension parser trusts the cluster it has just
read. A crafted image makes it read past the end of that cluster, and
with a large enough cluster size it makes the dirty bitmap code memset
memory which does not belong to it. All of it is reachable from
qemu-img info -f parallels image
on an untrusted image, with no guest and no write involved.
The reported problem
--------------------
parallels_parse_format_extension() advances its cursor over a feature
payload but subtracts only the feature header size from the number of
bytes it believes are left. A feature whose payload ends exactly at the
end of the cluster leaves the cursor at the end of the allocation while
the count still permits one more header, and the next iteration reads
it:
Invalid read of size 8
at memcpy (string_fortified.h:29)
by parallels_parse_format_extension (parallels-ext.c:250)
by parallels_read_format_extension (parallels-ext.c:313)
by parallels_open (parallels.c:1386)
Address 0x87831e0 is 0 bytes after a block of size 512 alloc'd
That is patch 1, reported privately on
https://gitlab.com/qemu-project/qemu/-/issues/4156.
The worse one
-------------
bdrv_dirty_bitmap_serialization_coverage() takes the chunk size as an
int and shifts it left by 3 before the result is widened, so a cluster
of 1 << 28 or above overflows. parallels_open() allows clusters up to
about 2 GiB, and with a bitmap needing two L1 entries the bogus limit
makes "bm_size - offset" underflow in parallels_load_bitmap_data().
Both wrong values pass the assertions in serialization_chunk() and the
resulting index lands outside the hbitmap:
#1 memset (__ch=255) at string_fortified.h:59
#2 hbitmap_deserialize_ones (start=18446742974197923840,
count=3298534883328) at hbitmap.c:753
#3 bdrv_dirty_bitmap_deserialize_ones (...) at dirty-bitmap.c:648
#4 parallels_load_bitmap_data (...) at parallels-ext.c:88
A 128 KiB image file is enough to get there. The target lands far
outside any mapping in the case I built, so what I can demonstrate is a
wild write and a crash rather than a controlled one, but it is a write
and not a read. Patch 3 fixes the overflow. Patch 4 refuses such an
offset in the parallels loop as well, since both the cluster size and
the L1 contents come from the image.
The rest
--------
Patch 2 validates the bitmap granularity, which reaches
bdrv_create_dirty_bitmap() unchecked and trips its assertion for a
granularity of zero, one which is not a power of two, or one which
wraps when shifted by BDRV_SECTOR_BITS. Patch 5 refuses an extension
cluster which does not lie inside the image file, patch 6 stops
aborting when that cluster cannot be allocated, and patch 7 is a leak.
Patch 3 is the only one outside the parallels driver and can be taken
on its own. The parallels patches do not depend on it, although without
it patch 4 is what keeps the memset from happening.
Testing
-------
Patch 8 extends tests/qemu-iotests/tests/parallels-read-bitmap with
hand crafted extension clusters. With this series reverted, four of the
cases abort in bdrv_create_dirty_bitmap(), one segfaults in
hbitmap_deserialize_ones() and one reports a feature header made of
heap bytes read past the cluster. parallels-read-bitmap,
parallels-checks, 076 and 118 pass, and every patch builds on its own.
No CVE has been requested.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Denis V. Lunev (8):
parallels: fix out-of-bounds read in format extension parsing
parallels: validate dirty bitmap granularity
dirty-bitmap: fix integer overflow in serialization coverage
parallels: bound the bitmap L1 table against the bitmap size
parallels: reject a Format Extension outside the image file
parallels: allocate the Format Extension cluster gracefully
parallels: fix GSList leak on the format extension success path
iotests: cover the Parallels format extension parser
block/dirty-bitmap.c | 2 +-
block/parallels-ext.c | 46 ++++-
block/parallels.c | 9 +-
.../qemu-iotests/tests/parallels-read-bitmap | 180 +++++++++++++++++-
.../tests/parallels-read-bitmap.out | 33 ++++
5 files changed, 259 insertions(+), 11 deletions(-)
base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
--
2.53.0