qemu-stable
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.0 19/47] qcow2: Check header


From: Max Reitz
Subject: Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.0 19/47] qcow2: Check header_length (CVE-2014-0144)
Date: Wed, 26 Mar 2014 21:40:12 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0

On 26.03.2014 13:05, Stefan Hajnoczi wrote:
From: Kevin Wolf <address@hidden>

This fixes an unbounded allocation for s->unknown_header_fields.

Signed-off-by: Kevin Wolf <address@hidden>
---
  block/qcow2.c              | 34 +++++++++++++++++++-------
  tests/qemu-iotests/080     | 61 ++++++++++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/080.out |  9 +++++++
  tests/qemu-iotests/group   |  1 +
  4 files changed, 96 insertions(+), 9 deletions(-)
  create mode 100755 tests/qemu-iotests/080
  create mode 100644 tests/qemu-iotests/080.out

diff --git a/block/qcow2.c b/block/qcow2.c
index b9dc960..c3c88e9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -460,6 +460,18 @@ static int qcow2_open(BlockDriverState *bs, QDict 
*options, int flags,
s->qcow_version = header.version; + /* Initialise cluster size */
+    if (header.cluster_bits < MIN_CLUSTER_BITS ||
+        header.cluster_bits > MAX_CLUSTER_BITS) {
+        error_setg(errp, "Unsupported cluster size: 2^%i", 
header.cluster_bits);
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    s->cluster_bits = header.cluster_bits;
+    s->cluster_size = 1 << s->cluster_bits;
+    s->cluster_sectors = 1 << (s->cluster_bits - 9);
+
      /* Initialise version 3 header fields */
      if (header.version == 2) {
          header.incompatible_features    = 0;
@@ -473,6 +485,18 @@ static int qcow2_open(BlockDriverState *bs, QDict 
*options, int flags,
          be64_to_cpus(&header.autoclear_features);
          be32_to_cpus(&header.refcount_order);
          be32_to_cpus(&header.header_length);
+
+        if (header.header_length < 104) {
+            error_setg(errp, "qcow2 header too short");

I remember having a small discussion about whether to do this check or not once. The point is that if the value of this field is less than 104, its value is automatically invalid, as it is right at the end of the mandatory header fields (bytes 100 to 103); but it's hard to write a proper easy-to-understand error message, therefore any discussion about this is pretty moot (as far as I remember, this is also the result the mentioned discussion yielded). It's basically just nitpicking.

+            ret = -EINVAL;
+            goto fail;
+        }
+    }
+
+    if (header.header_length > s->cluster_size) {
+        error_setg(errp, "qcow2 header exceeds cluster size");
+        ret = -EINVAL;
+        goto fail;
      }
if (header.header_length > sizeof(header)) {
@@ -529,12 +553,6 @@ static int qcow2_open(BlockDriverState *bs, QDict 
*options, int flags,
      }
      s->refcount_order = header.refcount_order;
- if (header.cluster_bits < MIN_CLUSTER_BITS ||
-        header.cluster_bits > MAX_CLUSTER_BITS) {
-        error_setg(errp, "Unsupported cluster size: 2^%i", 
header.cluster_bits);
-        ret = -EINVAL;
-        goto fail;
-    }
      if (header.crypt_method > QCOW_CRYPT_AES) {
          error_setg(errp, "Unsupported encryption method: %i",
                     header.crypt_method);
@@ -545,9 +563,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, 
int flags,
      if (s->crypt_method_header) {
          bs->encrypted = 1;
      }
-    s->cluster_bits = header.cluster_bits;
-    s->cluster_size = 1 << s->cluster_bits;
-    s->cluster_sectors = 1 << (s->cluster_bits - 9);
+
      s->l2_bits = s->cluster_bits - 3; /* L2 is always one cluster */
      s->l2_size = 1 << s->l2_bits;
      bs->total_sectors = header.size / 512;
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
new file mode 100755
index 0000000..6512701
--- /dev/null
+++ b/tests/qemu-iotests/080
@@ -0,0 +1,61 @@
+#!/bin/bash
+#
+# qcow2 format input validation tests
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
address@hidden
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1       # failure is the default!
+
+_cleanup()
+{
+       _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+header_size=104
+offset_header_size=100
+offset_ext_magic=$header_size
+offset_ext_size=$((header_size + 4))
+
+echo
+echo "== Huge header size =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_header_size" "\xff\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | 
_filter_testdir
+poke_file "$TEST_IMG" "$offset_header_size" "\x7f\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | 
_filter_testdir
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
new file mode 100644
index 0000000..41a166a
--- /dev/null
+++ b/tests/qemu-iotests/080.out
@@ -0,0 +1,9 @@
+QA output created by 080
+
+== Huge header size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-io: can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
+no file open, try 'help open'
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 9c99edc..ed44f35 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -85,6 +85,7 @@
  077 rw auto quick
  078 rw auto
  079 rw auto
+080 rw auto
  081 rw auto
  082 rw auto quick
  083 rw auto

Reviewed-by: Max Reitz <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]