[qemu] QCOW1 validation CVEs: CVE-2014-0222, CVE-2014-0223 (bz #1097232, bz #1097238, bz #1097222, bz #1097

Cole Robinson crobinso at fedoraproject.org
Sun Jun 1 00:49:59 UTC 2014


commit 12cd5461617934ff8450ab4e0eb2d0a31b8e125d
Author: Cole Robinson <crobinso at redhat.com>
Date:   Sat May 31 20:49:42 2014 -0400

    QCOW1 validation CVEs: CVE-2014-0222, CVE-2014-0223 (bz #1097232, bz #1097238, bz #1097222, bz #1097216)
    CVE-2014-3461: Issues in USB post load checks (bz #1097260, bz #1096821)

 ...qcow1-Make-padding-in-the-header-explicit.patch |   34 ++++++++++++
 0102-qcow1-Check-maximum-cluster-size.patch        |   48 ++++++++++++++++
 ...cow1-Validate-L2-table-size-CVE-2014-0222.patch |   48 ++++++++++++++++
 0104-qcow1-Validate-image-size-CVE-2014-0223.patch |   57 ++++++++++++++++++++
 ...-qcow1-Stricter-backing-file-length-check.patch |   48 ++++++++++++++++
 0106-usb-fix-up-post-load-checks.patch             |   37 +++++++++++++
 qemu.spec                                          |   31 +++++++++--
 7 files changed, 299 insertions(+), 4 deletions(-)
---
diff --git a/0101-qcow1-Make-padding-in-the-header-explicit.patch b/0101-qcow1-Make-padding-in-the-header-explicit.patch
new file mode 100644
index 0000000..723cc80
--- /dev/null
+++ b/0101-qcow1-Make-padding-in-the-header-explicit.patch
@@ -0,0 +1,34 @@
+From 709786ed4fa98cd281beaac3c6770292bd045a30 Mon Sep 17 00:00:00 2001
+From: Kevin Wolf <kwolf at redhat.com>
+Date: Wed, 7 May 2014 16:56:10 +0200
+Subject: [PATCH] qcow1: Make padding in the header explicit
+
+We were relying on all compilers inserting the same padding in the
+header struct that is used for the on-disk format. Let's not do that.
+Mark the struct as packed and insert an explicit padding field for
+compatibility.
+
+Cc: qemu-stable at nongnu.org
+Signed-off-by: Kevin Wolf <kwolf at redhat.com>
+Reviewed-by: Benoit Canet <benoit at irqsave.net>
+(cherry picked from commit ea54feff58efedc809641474b25a3130309678e7)
+---
+ block/qcow.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/block/qcow.c b/block/qcow.c
+index d5a7d5f..9018f44 100644
+--- a/block/qcow.c
++++ b/block/qcow.c
+@@ -48,9 +48,10 @@ typedef struct QCowHeader {
+     uint64_t size; /* in bytes */
+     uint8_t cluster_bits;
+     uint8_t l2_bits;
++    uint16_t padding;
+     uint32_t crypt_method;
+     uint64_t l1_table_offset;
+-} QCowHeader;
++} QEMU_PACKED QCowHeader;
+ 
+ #define L2_CACHE_SIZE 16
+ 
diff --git a/0102-qcow1-Check-maximum-cluster-size.patch b/0102-qcow1-Check-maximum-cluster-size.patch
new file mode 100644
index 0000000..071bd79
--- /dev/null
+++ b/0102-qcow1-Check-maximum-cluster-size.patch
@@ -0,0 +1,48 @@
+From 6893e96e6b58d809a08c6491f76df221fd1a6473 Mon Sep 17 00:00:00 2001
+From: Kevin Wolf <kwolf at redhat.com>
+Date: Wed, 7 May 2014 17:30:30 +0200
+Subject: [PATCH] qcow1: Check maximum cluster size
+
+Huge values for header.cluster_bits cause unbounded allocations (e.g.
+for s->cluster_cache) and crash qemu this way. Less huge values may
+survive those allocations, but can cause integer overflows later on.
+
+The only cluster sizes that qemu can create are 4k (for standalone
+images) and 512 (for images with backing files), so we can limit it
+to 64k.
+
+Cc: qemu-stable at nongnu.org
+Signed-off-by: Kevin Wolf <kwolf at redhat.com>
+Reviewed-by: Benoit Canet <benoit at irqsave.net>
+(cherry picked from commit 7159a45b2bf2dcb9f49f1e27d1d3d135a0247a2f)
+
+Conflicts:
+	tests/qemu-iotests/group
+---
+ block/qcow.c | 10 ++++++++--
+ 1 file changed, 8 insertions(+), 2 deletions(-)
+
+diff --git a/block/qcow.c b/block/qcow.c
+index 9018f44..26bb923 100644
+--- a/block/qcow.c
++++ b/block/qcow.c
+@@ -127,11 +127,17 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
+         goto fail;
+     }
+ 
+-    if (header.size <= 1 || header.cluster_bits < 9) {
+-        error_setg(errp, "invalid value in qcow header");
++    if (header.size <= 1) {
++        error_setg(errp, "Image size is too small (must be at least 2 bytes)");
+         ret = -EINVAL;
+         goto fail;
+     }
++    if (header.cluster_bits < 9 || header.cluster_bits > 16) {
++        error_setg(errp, "Cluster size must be between 512 and 64k");
++        ret = -EINVAL;
++        goto fail;
++    }
++
+     if (header.crypt_method > QCOW_CRYPT_AES) {
+         error_setg(errp, "invalid encryption method in qcow header");
+         ret = -EINVAL;
diff --git a/0103-qcow1-Validate-L2-table-size-CVE-2014-0222.patch b/0103-qcow1-Validate-L2-table-size-CVE-2014-0222.patch
new file mode 100644
index 0000000..db3b686
--- /dev/null
+++ b/0103-qcow1-Validate-L2-table-size-CVE-2014-0222.patch
@@ -0,0 +1,48 @@
+From 71ae37ec9806ab76afcdb40cf5f080af378848ac Mon Sep 17 00:00:00 2001
+From: Kevin Wolf <kwolf at redhat.com>
+Date: Thu, 15 May 2014 16:10:11 +0200
+Subject: [PATCH] qcow1: Validate L2 table size (CVE-2014-0222)
+
+Too large L2 table sizes cause unbounded allocations. Images actually
+created by qemu-img only have 512 byte or 4k L2 tables.
+
+To keep things consistent with cluster sizes, allow ranges between 512
+bytes and 64k (in fact, down to 1 entry = 8 bytes is technically
+working, but L2 table sizes smaller than a cluster don't make a lot of
+sense).
+
+This also means that the number of bytes on the virtual disk that are
+described by the same L2 table is limited to at most 8k * 64k or 2^29,
+preventively avoiding any integer overflows.
+
+Cc: qemu-stable at nongnu.org
+Signed-off-by: Kevin Wolf <kwolf at redhat.com>
+Reviewed-by: Benoit Canet <benoit at irqsave.net>
+(cherry picked from commit 42eb58179b3b215bb507da3262b682b8a2ec10b5)
+
+Conflicts:
+	tests/qemu-iotests/092
+	tests/qemu-iotests/092.out
+---
+ block/qcow.c | 8 ++++++++
+ 1 file changed, 8 insertions(+)
+
+diff --git a/block/qcow.c b/block/qcow.c
+index 26bb923..8718ca5 100644
+--- a/block/qcow.c
++++ b/block/qcow.c
+@@ -138,6 +138,14 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
+         goto fail;
+     }
+ 
++    /* l2_bits specifies number of entries; storing a uint64_t in each entry,
++     * so bytes = num_entries << 3. */
++    if (header.l2_bits < 9 - 3 || header.l2_bits > 16 - 3) {
++        error_setg(errp, "L2 table size must be between 512 and 64k");
++        ret = -EINVAL;
++        goto fail;
++    }
++
+     if (header.crypt_method > QCOW_CRYPT_AES) {
+         error_setg(errp, "invalid encryption method in qcow header");
+         ret = -EINVAL;
diff --git a/0104-qcow1-Validate-image-size-CVE-2014-0223.patch b/0104-qcow1-Validate-image-size-CVE-2014-0223.patch
new file mode 100644
index 0000000..547362a
--- /dev/null
+++ b/0104-qcow1-Validate-image-size-CVE-2014-0223.patch
@@ -0,0 +1,57 @@
+From 92e1dd206a3bb8ddbea0ece22bc05e9446a69436 Mon Sep 17 00:00:00 2001
+From: Kevin Wolf <kwolf at redhat.com>
+Date: Thu, 8 May 2014 13:08:20 +0200
+Subject: [PATCH] qcow1: Validate image size (CVE-2014-0223)
+
+A huge image size could cause s->l1_size to overflow. Make sure that
+images never require a L1 table larger than what fits in s->l1_size.
+
+This cannot only cause unbounded allocations, but also the allocation of
+a too small L1 table, resulting in out-of-bounds array accesses (both
+reads and writes).
+
+Cc: qemu-stable at nongnu.org
+Signed-off-by: Kevin Wolf <kwolf at redhat.com>
+(cherry picked from commit 46485de0cb357b57373e1ca895adedf1f3ed46ec)
+
+Conflicts:
+	tests/qemu-iotests/092
+	tests/qemu-iotests/092.out
+---
+ block/qcow.c | 16 ++++++++++++++--
+ 1 file changed, 14 insertions(+), 2 deletions(-)
+
+diff --git a/block/qcow.c b/block/qcow.c
+index 8718ca5..f9cb009 100644
+--- a/block/qcow.c
++++ b/block/qcow.c
+@@ -61,7 +61,7 @@ typedef struct BDRVQcowState {
+     int cluster_sectors;
+     int l2_bits;
+     int l2_size;
+-    int l1_size;
++    unsigned int l1_size;
+     uint64_t cluster_offset_mask;
+     uint64_t l1_table_offset;
+     uint64_t *l1_table;
+@@ -165,7 +165,19 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
+ 
+     /* read the level 1 table */
+     shift = s->cluster_bits + s->l2_bits;
+-    s->l1_size = (header.size + (1LL << shift) - 1) >> shift;
++    if (header.size > UINT64_MAX - (1LL << shift)) {
++        error_setg(errp, "Image too large");
++        ret = -EINVAL;
++        goto fail;
++    } else {
++        uint64_t l1_size = (header.size + (1LL << shift) - 1) >> shift;
++        if (l1_size > INT_MAX / sizeof(uint64_t)) {
++            error_setg(errp, "Image too large");
++            ret = -EINVAL;
++            goto fail;
++        }
++        s->l1_size = l1_size;
++    }
+ 
+     s->l1_table_offset = header.l1_table_offset;
+     s->l1_table = g_malloc(s->l1_size * sizeof(uint64_t));
diff --git a/0105-qcow1-Stricter-backing-file-length-check.patch b/0105-qcow1-Stricter-backing-file-length-check.patch
new file mode 100644
index 0000000..60894ed
--- /dev/null
+++ b/0105-qcow1-Stricter-backing-file-length-check.patch
@@ -0,0 +1,48 @@
+From deaa4693c8533862fdda9bf584c24d4f2ef50029 Mon Sep 17 00:00:00 2001
+From: Kevin Wolf <kwolf at redhat.com>
+Date: Thu, 8 May 2014 13:35:09 +0200
+Subject: [PATCH] qcow1: Stricter backing file length check
+
+Like qcow2 since commit 6d33e8e7, error out on invalid lengths instead
+of silently truncating them to 1023.
+
+Also don't rely on bdrv_pread() catching integer overflows that make len
+negative, but use unsigned variables in the first place.
+
+Cc: qemu-stable at nongnu.org
+Signed-off-by: Kevin Wolf <kwolf at redhat.com>
+Reviewed-by: Benoit Canet <benoit at irqsave.net>
+(cherry picked from commit d66e5cee002c471b78139228a4e7012736b375f9)
+
+Conflicts:
+	tests/qemu-iotests/092
+	tests/qemu-iotests/092.out
+---
+ block/qcow.c | 7 +++++--
+ 1 file changed, 5 insertions(+), 2 deletions(-)
+
+diff --git a/block/qcow.c b/block/qcow.c
+index f9cb009..c0a3b89 100644
+--- a/block/qcow.c
++++ b/block/qcow.c
+@@ -97,7 +97,8 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
+                      Error **errp)
+ {
+     BDRVQcowState *s = bs->opaque;
+-    int len, i, shift, ret;
++    unsigned int len, i, shift;
++    int ret;
+     QCowHeader header;
+ 
+     ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
+@@ -201,7 +202,9 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
+     if (header.backing_file_offset != 0) {
+         len = header.backing_file_size;
+         if (len > 1023) {
+-            len = 1023;
++            error_setg(errp, "Backing file name too long");
++            ret = -EINVAL;
++            goto fail;
+         }
+         ret = bdrv_pread(bs->file, header.backing_file_offset,
+                    bs->backing_file, len);
diff --git a/0106-usb-fix-up-post-load-checks.patch b/0106-usb-fix-up-post-load-checks.patch
new file mode 100644
index 0000000..3f0c217
--- /dev/null
+++ b/0106-usb-fix-up-post-load-checks.patch
@@ -0,0 +1,37 @@
+From 40e49e4fab60b3b323263f06b7a8385fa9b62e89 Mon Sep 17 00:00:00 2001
+From: "Michael S. Tsirkin" <mst at redhat.com>
+Date: Tue, 13 May 2014 12:33:16 +0300
+Subject: [PATCH] usb: fix up post load checks
+
+Correct post load checks:
+1. dev->setup_len == sizeof(dev->data_buf)
+    seems fine, no need to fail migration
+2. When state is DATA, passing index > len
+   will cause memcpy with negative length,
+   resulting in heap overflow
+
+First of the issues was reported by dgilbert.
+
+Reported-by: "Dr. David Alan Gilbert" <dgilbert at redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
+Signed-off-by: Juan Quintela <quintela at redhat.com>
+(cherry picked from commit 719ffe1f5f72b1c7ace4afe9ba2815bcb53a829e)
+---
+ hw/usb/bus.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/hw/usb/bus.c b/hw/usb/bus.c
+index e48b19f..ff1dfe6 100644
+--- a/hw/usb/bus.c
++++ b/hw/usb/bus.c
+@@ -51,8 +51,8 @@ static int usb_device_post_load(void *opaque, int version_id)
+     }
+     if (dev->setup_index < 0 ||
+         dev->setup_len < 0 ||
+-        dev->setup_index >= sizeof(dev->data_buf) ||
+-        dev->setup_len >= sizeof(dev->data_buf)) {
++        dev->setup_index > dev->setup_len ||
++        dev->setup_len > sizeof(dev->data_buf)) {
+         return -EINVAL;
+     }
+     return 0;
diff --git a/qemu.spec b/qemu.spec
index 0194f38..b98cb37 100644
--- a/qemu.spec
+++ b/qemu.spec
@@ -226,6 +226,17 @@ Patch0022: 0022-openpic-avoid-buffer-overrun-on-incoming-migration.patch
 Patch0023: 0023-virtio-net-out-of-bounds-buffer-write-on-load.patch
 Patch0024: 0024-virtio-validate-config_len-on-load.patch
 
+# QCOW1 validation CVEs: CVE-2014-0222, CVE-2014-0223 (bz #1097232, bz
+# #1097238, bz #1097222, bz #1097216)
+Patch0101: 0101-qcow1-Make-padding-in-the-header-explicit.patch
+Patch0102: 0102-qcow1-Check-maximum-cluster-size.patch
+Patch0103: 0103-qcow1-Validate-L2-table-size-CVE-2014-0222.patch
+Patch0104: 0104-qcow1-Validate-image-size-CVE-2014-0223.patch
+Patch0105: 0105-qcow1-Stricter-backing-file-length-check.patch
+# CVE-2014-3461: Issues in USB post load checks (bz #1097260, bz
+# #1096821)
+Patch0106: 0106-usb-fix-up-post-load-checks.patch
+
 BuildRequires: SDL-devel
 BuildRequires: zlib-devel
 BuildRequires: which
@@ -775,6 +786,17 @@ CAC emulation development files.
 %patch0023 -p1
 %patch0024 -p1
 
+# QCOW1 validation CVEs: CVE-2014-0222, CVE-2014-0223 (bz #1097232, bz
+# #1097238, bz #1097222, bz #1097216)
+%patch0101 -p1
+%patch0102 -p1
+%patch0103 -p1
+%patch0104 -p1
+%patch0105 -p1
+# CVE-2014-3461: Issues in USB post load checks (bz #1097260, bz
+# #1096821)
+%patch0106 -p1
+
 
 %build
 %if %{with kvmonly}
@@ -1099,8 +1121,7 @@ done
 # Disabled on aarch64 where it fails with several errors.  Will
 # investigate and fix when we have access to real hardware - RWMJ.
 # 2014-03-24: Suddenly failing on arm32 as well - crobinso
-# 2014-05-24: disable test on s390 (#1100971) - DH
-%ifnarch aarch64 s390
+%ifnarch aarch64
 make check
 %endif
 
@@ -1542,8 +1563,10 @@ getent passwd qemu >/dev/null || \
 %endif
 
 %changelog
-* Sat May 24 2014 Dan HorĂ¡k <dan[at]danny.cz> - 2:2.0.0-5
-- Disable tests on s390 (#1100971)
+* Sat May 31 2014 Cole Robinson <crobinso at redhat.com> - 2:2.0.0-5
+- QCOW1 validation CVEs: CVE-2014-0222, CVE-2014-0223 (bz #1097232, bz
+  #1097238, bz #1097222, bz #1097216)
+- CVE-2014-3461: Issues in USB post load checks (bz #1097260, bz #1096821)
 
 * Sun May 11 2014 Cole Robinson <crobinso at redhat.com> - 2:2.0.0-4
 - Migration CVEs: CVE-2014-0182 etc.


More information about the scm-commits mailing list