From 12cd5461617934ff8450ab4e0eb2d0a31b8e125d Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Jun 01 2014 00:49:56 +0000 Subject: 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) --- 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 +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@nongnu.org +Signed-off-by: Kevin Wolf +Reviewed-by: Benoit Canet +(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 +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@nongnu.org +Signed-off-by: Kevin Wolf +Reviewed-by: Benoit Canet +(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 +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@nongnu.org +Signed-off-by: Kevin Wolf +Reviewed-by: Benoit Canet +(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 +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@nongnu.org +Signed-off-by: Kevin Wolf +(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 +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@nongnu.org +Signed-off-by: Kevin Wolf +Reviewed-by: Benoit Canet +(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" +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" +Signed-off-by: Michael S. Tsirkin +Signed-off-by: Juan Quintela +(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 - 2:2.0.0-5 -- Disable tests on s390 (#1100971) +* Sat May 31 2014 Cole Robinson - 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 - 2:2.0.0-4 - Migration CVEs: CVE-2014-0182 etc.