From f602c82d48f08db71441b348ac483c3bd158307d Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Sat, 31 May 2014 21:01:22 -0400 Subject: [PATCH] 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) --- ...-Make-padding-in-the-header-explicit.patch | 34 +++++++++++ 0402-qcow1-Check-maximum-cluster-size.patch | 47 +++++++++++++++ ...Validate-L2-table-size-CVE-2014-0222.patch | 48 ++++++++++++++++ ...w1-Validate-image-size-CVE-2014-0223.patch | 57 +++++++++++++++++++ ...1-Stricter-backing-file-length-check.patch | 48 ++++++++++++++++ 0406-usb-fix-up-post-load-checks.patch | 37 ++++++++++++ qemu.spec | 29 +++++++++- 7 files changed, 299 insertions(+), 1 deletion(-) create mode 100644 0401-qcow1-Make-padding-in-the-header-explicit.patch create mode 100644 0402-qcow1-Check-maximum-cluster-size.patch create mode 100644 0403-qcow1-Validate-L2-table-size-CVE-2014-0222.patch create mode 100644 0404-qcow1-Validate-image-size-CVE-2014-0223.patch create mode 100644 0405-qcow1-Stricter-backing-file-length-check.patch create mode 100644 0406-usb-fix-up-post-load-checks.patch diff --git a/0401-qcow1-Make-padding-in-the-header-explicit.patch b/0401-qcow1-Make-padding-in-the-header-explicit.patch new file mode 100644 index 0000000..137d904 --- /dev/null +++ b/0401-qcow1-Make-padding-in-the-header-explicit.patch @@ -0,0 +1,34 @@ +From a62cd901b526bd7bc8bddd8c6a2140cf2570bb09 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 5239bd6..ca52464 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/0402-qcow1-Check-maximum-cluster-size.patch b/0402-qcow1-Check-maximum-cluster-size.patch new file mode 100644 index 0000000..4ae1f8c --- /dev/null +++ b/0402-qcow1-Check-maximum-cluster-size.patch @@ -0,0 +1,47 @@ +From 0e6acac62081eaf7927f109bf548da1d67f8a6dd 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 | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/block/qcow.c b/block/qcow.c +index ca52464..2379132 100644 +--- a/block/qcow.c ++++ b/block/qcow.c +@@ -125,10 +125,17 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags) + goto fail; + } + +- if (header.size <= 1 || header.cluster_bits < 9) { ++ if (header.size <= 1) { ++ error_report("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_report("Cluster size must be between 512 and 64k"); ++ ret = -EINVAL; ++ goto fail; ++ } ++ + if (header.crypt_method > QCOW_CRYPT_AES) { + ret = -EINVAL; + goto fail; diff --git a/0403-qcow1-Validate-L2-table-size-CVE-2014-0222.patch b/0403-qcow1-Validate-L2-table-size-CVE-2014-0222.patch new file mode 100644 index 0000000..d3e4660 --- /dev/null +++ b/0403-qcow1-Validate-L2-table-size-CVE-2014-0222.patch @@ -0,0 +1,48 @@ +From bacbfafc865397fa29a30bfebf9e5a283efae505 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 2379132..bdd2003 100644 +--- a/block/qcow.c ++++ b/block/qcow.c +@@ -136,6 +136,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_report("L2 table size must be between 512 and 64k"); ++ ret = -EINVAL; ++ goto fail; ++ } ++ + if (header.crypt_method > QCOW_CRYPT_AES) { + ret = -EINVAL; + goto fail; diff --git a/0404-qcow1-Validate-image-size-CVE-2014-0223.patch b/0404-qcow1-Validate-image-size-CVE-2014-0223.patch new file mode 100644 index 0000000..fb9c8c1 --- /dev/null +++ b/0404-qcow1-Validate-image-size-CVE-2014-0223.patch @@ -0,0 +1,57 @@ +From 858341dc5d0b31bf7310034a9a4e8773984f9c52 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 bdd2003..4946bbf 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; +@@ -162,7 +162,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_report("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_report("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/0405-qcow1-Stricter-backing-file-length-check.patch b/0405-qcow1-Stricter-backing-file-length-check.patch new file mode 100644 index 0000000..6254bb4 --- /dev/null +++ b/0405-qcow1-Stricter-backing-file-length-check.patch @@ -0,0 +1,48 @@ +From 1c108dfb4997c510d944c2fecc5b7eff7949f2af 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 4946bbf..9439c73 100644 +--- a/block/qcow.c ++++ b/block/qcow.c +@@ -96,7 +96,8 @@ static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename) + static int qcow_open(BlockDriverState *bs, QDict *options, int flags) + { + 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)); +@@ -198,7 +199,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_report("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/0406-usb-fix-up-post-load-checks.patch b/0406-usb-fix-up-post-load-checks.patch new file mode 100644 index 0000000..46173c6 --- /dev/null +++ b/0406-usb-fix-up-post-load-checks.patch @@ -0,0 +1,37 @@ +From 466ded7c745771009cd00a9a14000476691e2498 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 c6c2005..8a1edcc 100644 +--- a/hw/usb/bus.c ++++ b/hw/usb/bus.c +@@ -49,8 +49,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 cd68619..1349f27 100644 --- a/qemu.spec +++ b/qemu.spec @@ -139,7 +139,7 @@ Summary: QEMU is a FAST! processor emulator Name: qemu Version: 1.6.2 -Release: 5%{?dist} +Release: 6%{?dist} Epoch: 2 License: GPLv2+ and LGPLv2+ and BSD Group: Development/Tools @@ -337,6 +337,17 @@ Patch0321: 0321-openpic-avoid-buffer-overrun-on-incoming-migration.patch Patch0322: 0322-virtio-net-out-of-bounds-buffer-write-on-load.patch Patch0323: 0323-virtio-validate-config_len-on-load.patch +# QCOW1 validation CVEs: CVE-2014-0222, CVE-2014-0223 (bz #1097232, bz +# #1097238, bz #1097222, bz #1097216) +Patch0401: 0401-qcow1-Make-padding-in-the-header-explicit.patch +Patch0402: 0402-qcow1-Check-maximum-cluster-size.patch +Patch0403: 0403-qcow1-Validate-L2-table-size-CVE-2014-0222.patch +Patch0404: 0404-qcow1-Validate-image-size-CVE-2014-0223.patch +Patch0405: 0405-qcow1-Stricter-backing-file-length-check.patch +# CVE-2014-3461: Issues in USB post load checks (bz #1097260, bz +# #1096821) +Patch0406: 0406-usb-fix-up-post-load-checks.patch + BuildRequires: SDL-devel BuildRequires: zlib-devel BuildRequires: which @@ -1005,6 +1016,17 @@ CAC emulation development files. %patch0322 -p1 %patch0323 -p1 +# QCOW1 validation CVEs: CVE-2014-0222, CVE-2014-0223 (bz #1097232, bz +# #1097238, bz #1097222, bz #1097216) +%patch0401 -p1 +%patch0402 -p1 +%patch0403 -p1 +%patch0404 -p1 +%patch0405 -p1 +# CVE-2014-3461: Issues in USB post load checks (bz #1097260, bz +# #1096821) +%patch0406 -p1 + %build %if %{with kvmonly} @@ -1711,6 +1733,11 @@ getent passwd qemu >/dev/null || \ %endif %changelog +* Sat May 31 2014 Cole Robinson - 2:1.6.2-6 +- 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:1.6.2-5 - Migration CVEs: CVE-2014-0182 etc.