[PATCH 0/5] block/vmdk: Fix various bugs in handling malicious image files

[PATCH 0/5] block/vmdk: Fix various bugs in handling malicious image files

[Peter Maydell]

This set of patches fixes four reported bugs in vmdk's handling of
potentially malicious image files:

 #3843: if cluster_sectors is 0x200000 we can overflow an 'int' and try
        to g_malloc() 0xffffffff80000000
 #4017: if cluster_sectors is 0 we will divide by zero and SIGFPE
 #4029: we can overflow a 32*64 multiply; we catch it a bit later on
        so there are no ill-effects, but for robustness it's better to
        check before the multiply
 #4571: we can read off the end of a block of memory when trying to
        parse descriptors; worst case is that we read some junk from
        the heap into BlockDriverState::auto_backing_file and
        ::backing_file. I don't think this is an exploitable info
        leak into the guest.
 #4571: if the input file specifies no extents, we hit an assert
        in vmdk_co_get_info() rather than rejecting the file
        (mentioned in passing in the same issue as previous bug)

I think these all classify as "hardening bugs" rather than security
issues, since they're just "self-denial-of-service" at worst.

Tested using qemu-iotests on the vmdk format.

thanks
-- PMM

Peter Maydell (5):
  block/vmdk: Avoid assertion if image file has no extents
  block/vmdk: Avoid reading off the end of descriptor memory
  block/vmdk: Use larger types for byte counts in vmdk_read_extent()
  block/vmdk: Reject a zero cluster_sectors parameter
  block/vmdk: Avoid overflowing multiplication in vmdk_open_vmdk4()

 block/vmdk.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH 1/5] block/vmdk: Avoid assertion if image file has no extents

[Peter Maydell]

On a malformed image file which has no extents in it, we will open the file
but then fall over on an assertion in vmdk_co_get_info():

    $ cat bad.vmdk
    # Disk DescriptorFile
    version=1
    CID=fffffffe
    parentCID=ffffffff
    createType="vmfs"

    $ qemu-img info -f vmdk /tmp/zz9
    qemu-img: ../../block/vmdk.c:3104: int vmdk_co_get_info(BlockDriverState *, BlockDriverInfo *): Assertion `s->num_extents' failed.
    Aborted (core dumped)

(This was noted in passing in the report of #4571.)

Cc: qemu-stable@nongnu.org
Fixes: 7fa60fa3778f1c7336 ("VMDK: open/read/write for monolithicFlat image")
Reported-by: router0mail <https://gitlab.com/router0mail>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 block/vmdk.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index b6e78a2b5d9..86059e1fa08 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1301,6 +1301,12 @@ vmdk_parse_extents(const char *desc, BlockDriverState *bs, QDict *options,
         extent->type = g_strdup(type);
     }
 
+    if (s->num_extents == 0) {
+        error_setg(errp, "Invalid file: no extent lines present");
+        ret = -EINVAL;
+        goto out;
+    }
+
     ret = 0;
     goto out;
 
-- 
2.43.0

[PATCH 2/5] block/vmdk: Avoid reading off the end of descriptor memory

[Peter Maydell]

In vmdk_parent_open() and vmdk_read_cid(), we read DESC_SIZE bytes
from the image file into a malloc'd buffer, and then look through it
for a keyword using strstr().  If found, we skip past the keyword and
attempt to parse a value from the next part of the buffer.  However,
we failed to check whether the end of the keyword is right at the end
of the buffer, and so a malicious image can cause us to read off the
end when parsing the value part.

Check for this condition before attempting to parse the value.

The worst case here is that the image causes us to read some junk
from the heap into the BlockDriverState backing_file and
auto_backing_file fields; the image could control those anyway, and
it's unlikely that this heap data could e.g. be leaked to the guest.

Cc: qemu-stable@nongnu.org
Reported-by: router0mail <https://gitlab.com/router0mail>
Fixes: 5f4da8c0f3736b2 ("New multiple snapshot support for VMDK")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4571
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 block/vmdk.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index 86059e1fa08..6e4e0423dc9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -337,6 +337,10 @@ vmdk_read_cid(BlockDriverState *bs, int parent, uint32_t *pcid)
         goto out;
     }
     p_name += cid_str_size;
+    if (p_name >= desc + DESC_SIZE) {
+        ret = -EINVAL;
+        goto out;
+    }
     if (sscanf(p_name, "%" SCNx32, &cid) != 1) {
         ret = -EINVAL;
         goto out;
@@ -508,6 +512,10 @@ static int GRAPH_RDLOCK vmdk_parent_open(BlockDriverState *bs)
         char *end_name;
 
         p_name += sizeof("parentFileNameHint") + 1;
+        if (p_name >= desc + DESC_SIZE + 1) {
+            ret = -EINVAL;
+            goto out;
+        }
         end_name = strchr(p_name, '\"');
         if (end_name == NULL) {
             ret = -EINVAL;
-- 
2.43.0

[PATCH 3/5] block/vmdk: Use larger types for byte counts in vmdk_read_extent()

[Peter Maydell]

In vmdk_read_extent() we use 'int' for cluster_bytes and buf_bytes.
However, cluster_bytes is extent->cluster_sectors * 512 and buf_bytes
is cluster_bytes * 2.  The maximum value for extent->cluster_sectors
(enforced in vmdk_add_extent()) is 0x20_0000, and so the worst case
for buf_bytes is 0x8000_0000, which overflows a signed int.

Make the variable types here int64_t, which matches the type used by
bdrv_co_pread()'s bytes argument and by extent->cluster_sectors.

The worst case thing that a malicious VMDK image could do is cause us
to try to g_malloc() 0xffffffff80000000, which will cause QEMU to
abort(), so this is a self-denial-of-service only, and so a hardening
bug rather than a security issue.

Cc: qemu-stable@nongnu.org
Fixes: 2b2c8c5dec6ba ("VMDK: read/write compressed extent")
Reported-by: boy juju <agx1657748706@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3843
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 block/vmdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 6e4e0423dc9..bd18d32085f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1940,7 +1940,7 @@ vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
                  int64_t offset_in_cluster, QEMUIOVector *qiov, int bytes)
 {
     int ret;
-    int cluster_bytes, buf_bytes;
+    int64_t cluster_bytes, buf_bytes;
     uint8_t *cluster_buf, *compressed_data;
     uint8_t *uncomp_buf;
     uint32_t data_len;
-- 
2.43.0

[PATCH 4/5] block/vmdk: Reject a zero cluster_sectors parameter

[Peter Maydell]

In vmdk_add_extent() we reject image files with an excessively large
cluster_sectors parameter, but have no lower bound.  In particular,
if the cluster_sectors is 0 then we will set l1_entry_sectors also to
0, and later crash with a SIGFPE for division by zero.

Reject a zero cluster_sectors value, unless this is a flat extent (in
which case cluster_sectors isn't relevant and the callsite always
passes us zero).

The worst case here is a self-denial-of-service with a QEMU crash,
so this is a hardening bug.

Cc: qemu-stable@nongnu.org
Fixes: b3976d3c8b89 ("VMDK: introduce VmdkExtent")
Reported-by: Tristan <https://gitlab.com/TristanInSec>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4017
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 block/vmdk.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index bd18d32085f..d7e23ae8098 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -552,6 +552,11 @@ static int vmdk_add_extent(BlockDriverState *bs,
     BDRVVmdkState *s = bs->opaque;
     int64_t nb_sectors;
 
+    if (!flat && cluster_sectors == 0) {
+        error_setg(errp, "Invalid zero granularity, image may be corrupt");
+        return -EINVAL;
+    }
+
     if (cluster_sectors > 0x200000) {
         /* 0x200000 * 512Bytes = 1GB for one cluster is unrealistic */
         error_setg(errp, "Invalid granularity, image may be corrupt");
-- 
2.43.0

[PATCH 5/5] block/vmdk: Avoid overflowing multiplication in vmdk_open_vmdk4()

[Peter Maydell]

In vmdk_open_vmdk4() we set the uint32_t l1_entry_sectors with:
    l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gt)
                        * le64_to_cpu(header.granularity);

We have checked that num_gtes_per_gt is at most 512, but the
header.granularity has not been checked at this point, and so the
multiply can overflow the 32-bit variable.  This overflow is harmless
beacuse we're about to call vmdk_add_extent(), which will fail if the
header.granularity (cluster_sectors) is more than 0x20_0000, but it's
a potential trap for the future if code that relies on the
l1_entry_sectors value gets added between where we set it and where
we make that check.

Check for overlarge granularity at the point of multiplication so
it's clear we can't overflow.

Cc: qemu-stable@nongnu.org
Reported-by: Tristan <https://gitlab.com/TristanInSec>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4029
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 block/vmdk.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index d7e23ae8098..a9bd8a4245f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1052,7 +1052,10 @@ vmdk_open_vmdk4(BlockDriverState *bs, BdrvChild *file, int flags,
         error_setg(errp, "L2 table size too big");
         return -EINVAL;
     }
-
+    if (le64_to_cpu(header.granularity) > 0x200000) {
+        error_setg(errp, "Invalid granularity, image may be corrupt");
+        return -EFBIG;
+    }
     l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gt)
                         * le64_to_cpu(header.granularity);
     if (l1_entry_sectors == 0) {
-- 
2.43.0

Re: [PATCH 0/5] block/vmdk: Fix various bugs in handling malicious image files

[Kevin Wolf]

Am 03.11.2026 um 15:35 hat Peter Maydell geschrieben:
> This set of patches fixes four reported bugs in vmdk's handling of
> potentially malicious image files:
> 
>  #3843: if cluster_sectors is 0x200000 we can overflow an 'int' and try
>         to g_malloc() 0xffffffff80000000
>  #4017: if cluster_sectors is 0 we will divide by zero and SIGFPE
>  #4029: we can overflow a 32*64 multiply; we catch it a bit later on
>         so there are no ill-effects, but for robustness it's better to
>         check before the multiply
>  #4571: we can read off the end of a block of memory when trying to
>         parse descriptors; worst case is that we read some junk from
>         the heap into BlockDriverState::auto_backing_file and
>         ::backing_file. I don't think this is an exploitable info
>         leak into the guest.
>  #4571: if the input file specifies no extents, we hit an assert
>         in vmdk_co_get_info() rather than rejecting the file
>         (mentioned in passing in the same issue as previous bug)
> 
> I think these all classify as "hardening bugs" rather than security
> issues, since they're just "self-denial-of-service" at worst.
> 
> Tested using qemu-iotests on the vmdk format.

Thanks, applied to the block branch.

It would have been nice to add new qemu-iotests cases that can catch
these bugs, maybe you still want to do that on top?

And just looking briefly at each of the places that are patched, it's
immediately clear to me that some more fixes are needed.  :-)

Kevin

Re: [PATCH 0/5] block/vmdk: Fix various bugs in handling malicious image files

[Peter Maydell]

On Thu, 5 Nov 2026 at 17:18, Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 03.11.2026 um 15:35 hat Peter Maydell geschrieben:
> > This set of patches fixes four reported bugs in vmdk's handling of
> > potentially malicious image files:
> >
> >  #3843: if cluster_sectors is 0x200000 we can overflow an 'int' and try
> >         to g_malloc() 0xffffffff80000000
> >  #4017: if cluster_sectors is 0 we will divide by zero and SIGFPE
> >  #4029: we can overflow a 32*64 multiply; we catch it a bit later on
> >         so there are no ill-effects, but for robustness it's better to
> >         check before the multiply
> >  #4571: we can read off the end of a block of memory when trying to
> >         parse descriptors; worst case is that we read some junk from
> >         the heap into BlockDriverState::auto_backing_file and
> >         ::backing_file. I don't think this is an exploitable info
> >         leak into the guest.
> >  #4571: if the input file specifies no extents, we hit an assert
> >         in vmdk_co_get_info() rather than rejecting the file
> >         (mentioned in passing in the same issue as previous bug)
> >
> > I think these all classify as "hardening bugs" rather than security
> > issues, since they're just "self-denial-of-service" at worst.
> >
> > Tested using qemu-iotests on the vmdk format.
>
> Thanks, applied to the block branch.
>
> It would have been nice to add new qemu-iotests cases that can catch
> these bugs, maybe you still want to do that on top?

I had a look at this, but then re-reading the bug reports I kind
of lost the will to continue trying to dig through the AI generated
report slop to try to extract something resembling a repro case
that could be put into the iotests. I'm also generally dubious
about the utility of adding regression tests for "we do something
wrong on this specific invalid input" because it seems unlikely to
me that we reintroduce the same bug again.

The one that was reasonably doable was the "no extent lines" bug.

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 49c58ad05c3..79fd4de1e7e 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -100,6 +100,18 @@ RW 12582912 VMFS "dummy.vmdk" 1
 EOF
 _img_info

+echo
+echo "=== Testing malformed VMFS with no extent lines ==="
+cat >"$TEST_IMG" <<EOF
+# Disk DescriptorFile
+version=1
+CID=fffffffe
+parentCID=ffffffff
+createType="vmfs"
+
+EOF
+_img_info
+
 echo
 echo "=== Testing truncated sparse ==="
 _make_test_img -o "subformat=monolithicSparse" 100G
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 275ee7c7781..56bdc085862 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2040,6 +2040,9 @@ read 512/512 bytes at offset 1063004405760
 === Testing malformed VMFS extent description line ===
 qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent line: RW
12582912 VMFS "dummy.IMGFMT" 1

+=== Testing malformed VMFS with no extent lines ===
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid file: no extent
lines present
+
 === Testing truncated sparse ===
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400
 qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File truncated,
expecting at least 13172736 bytes


thanks
-- PMM