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)
This commit is contained in:
Cole Robinson 2014-05-31 20:49:42 -04:00
parent 660494c491
commit 12cd546161
7 changed files with 299 additions and 4 deletions

View File

@ -0,0 +1,34 @@
From 709786ed4fa98cd281beaac3c6770292bd045a30 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@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@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoit Canet <benoit@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

View File

@ -0,0 +1,48 @@
From 6893e96e6b58d809a08c6491f76df221fd1a6473 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@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@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoit Canet <benoit@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;

View File

@ -0,0 +1,48 @@
From 71ae37ec9806ab76afcdb40cf5f080af378848ac Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@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@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoit Canet <benoit@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;

View File

@ -0,0 +1,57 @@
From 92e1dd206a3bb8ddbea0ece22bc05e9446a69436 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@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@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@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));

View File

@ -0,0 +1,48 @@
From deaa4693c8533862fdda9bf584c24d4f2ef50029 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@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@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoit Canet <benoit@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);

View File

@ -0,0 +1,37 @@
From 40e49e4fab60b3b323263f06b7a8385fa9b62e89 Mon Sep 17 00:00:00 2001
From: "Michael S. Tsirkin" <mst@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@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Juan Quintela <quintela@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;

View File

@ -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@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@redhat.com> - 2:2.0.0-4
- Migration CVEs: CVE-2014-0182 etc.