9ae3a8
From cba6c4e21a8dce857af0ed08d26801ebd5d5598a Mon Sep 17 00:00:00 2001
9ae3a8
From: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
Date: Tue, 25 Mar 2014 14:23:14 +0100
9ae3a8
Subject: [PATCH 07/49] block/cloop: refuse images with bogus offsets (CVE-2014-0144)
9ae3a8
9ae3a8
RH-Author: Kevin Wolf <kwolf@redhat.com>
9ae3a8
Message-id: <1395753835-7591-8-git-send-email-kwolf@redhat.com>
9ae3a8
Patchwork-id: n/a
9ae3a8
O-Subject: [virt-devel] [EMBARGOED RHEL-7.0 qemu-kvm PATCH 07/48] block/cloop: refuse images with bogus offsets (CVE-2014-0144)
9ae3a8
Bugzilla: 1079455
9ae3a8
RH-Acked-by: Jeff Cody <jcody@redhat.com>
9ae3a8
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
9ae3a8
9ae3a8
From: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
9ae3a8
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1079455
9ae3a8
Upstream status: Embargoed
9ae3a8
9ae3a8
The offsets[] array allows efficient seeking and tells us the maximum
9ae3a8
compressed data size. If the offsets are bogus the maximum compressed
9ae3a8
data size will be unrealistic.
9ae3a8
9ae3a8
This could cause g_malloc() to abort and bogus offsets mean the image is
9ae3a8
broken anyway. Therefore we should refuse such images.
9ae3a8
9ae3a8
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
9ae3a8
---
9ae3a8
 block/cloop.c              |   34 +++++++++++++++++++++++++++++-----
9ae3a8
 tests/qemu-iotests/075     |   15 +++++++++++++++
9ae3a8
 tests/qemu-iotests/075.out |    8 ++++++++
9ae3a8
 3 files changed, 52 insertions(+), 5 deletions(-)
9ae3a8
9ae3a8
diff --git a/block/cloop.c b/block/cloop.c
9ae3a8
index 844665e..55a804f 100644
9ae3a8
--- a/block/cloop.c
9ae3a8
+++ b/block/cloop.c
9ae3a8
@@ -124,12 +124,36 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
9ae3a8
     }
9ae3a8
 
9ae3a8
     for(i=0;i<s->n_blocks;i++) {
9ae3a8
+        uint64_t size;
9ae3a8
+
9ae3a8
         s->offsets[i] = be64_to_cpu(s->offsets[i]);
9ae3a8
-        if (i > 0) {
9ae3a8
-            uint32_t size = s->offsets[i] - s->offsets[i - 1];
9ae3a8
-            if (size > max_compressed_block_size) {
9ae3a8
-                max_compressed_block_size = size;
9ae3a8
-            }
9ae3a8
+        if (i == 0) {
9ae3a8
+            continue;
9ae3a8
+        }
9ae3a8
+
9ae3a8
+        if (s->offsets[i] < s->offsets[i - 1]) {
9ae3a8
+            error_setg(errp, "offsets not monotonically increasing at "
9ae3a8
+                       "index %u, image file is corrupt", i);
9ae3a8
+            ret = -EINVAL;
9ae3a8
+            goto fail;
9ae3a8
+        }
9ae3a8
+
9ae3a8
+        size = s->offsets[i] - s->offsets[i - 1];
9ae3a8
+
9ae3a8
+        /* Compressed blocks should be smaller than the uncompressed block size
9ae3a8
+         * but maybe compression performed poorly so the compressed block is
9ae3a8
+         * actually bigger.  Clamp down on unrealistic values to prevent
9ae3a8
+         * ridiculous s->compressed_block allocation.
9ae3a8
+         */
9ae3a8
+        if (size > 2 * MAX_BLOCK_SIZE) {
9ae3a8
+            error_setg(errp, "invalid compressed block size at index %u, "
9ae3a8
+                       "image file is corrupt", i);
9ae3a8
+            ret = -EINVAL;
9ae3a8
+            goto fail;
9ae3a8
+        }
9ae3a8
+
9ae3a8
+        if (size > max_compressed_block_size) {
9ae3a8
+            max_compressed_block_size = size;
9ae3a8
         }
9ae3a8
     }
9ae3a8
 
9ae3a8
diff --git a/tests/qemu-iotests/075 b/tests/qemu-iotests/075
9ae3a8
index 9c00fa8..d74fb33 100755
9ae3a8
--- a/tests/qemu-iotests/075
9ae3a8
+++ b/tests/qemu-iotests/075
9ae3a8
@@ -44,6 +44,7 @@ _supported_os Linux
9ae3a8
 
9ae3a8
 block_size_offset=128
9ae3a8
 n_blocks_offset=132
9ae3a8
+offsets_offset=136
9ae3a8
 
9ae3a8
 echo
9ae3a8
 echo "== check that the first sector can be read =="
9ae3a8
@@ -80,6 +81,20 @@ _use_sample_img simple-pattern.cloop.bz2
9ae3a8
 poke_file "$TEST_IMG" "$n_blocks_offset" "\x04\x00\x00\x01"
9ae3a8
 $QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
9ae3a8
 
9ae3a8
+echo
9ae3a8
+echo "== refuse images with non-monotonically increasing offsets =="
9ae3a8
+_use_sample_img simple-pattern.cloop.bz2
9ae3a8
+poke_file "$TEST_IMG" "$offsets_offset" "\x00\x00\x00\x00\xff\xff\xff\xff"
9ae3a8
+poke_file "$TEST_IMG" $((offsets_offset + 8)) "\x00\x00\x00\x00\xff\xfe\x00\x00"
9ae3a8
+$QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
9ae3a8
+
9ae3a8
+echo
9ae3a8
+echo "== refuse images with invalid compressed block size =="
9ae3a8
+_use_sample_img simple-pattern.cloop.bz2
9ae3a8
+poke_file "$TEST_IMG" "$offsets_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
9ae3a8
+poke_file "$TEST_IMG" $((offsets_offset + 8)) "\xff\xff\xff\xff\xff\xff\xff\xff"
9ae3a8
+$QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
9ae3a8
+
9ae3a8
 # success, all done
9ae3a8
 echo "*** done"
9ae3a8
 rm -f $seq.full
9ae3a8
diff --git a/tests/qemu-iotests/075.out b/tests/qemu-iotests/075.out
9ae3a8
index 7cdaee1..911cd3b 100644
9ae3a8
--- a/tests/qemu-iotests/075.out
9ae3a8
+++ b/tests/qemu-iotests/075.out
9ae3a8
@@ -23,4 +23,12 @@ no file open, try 'help open'
9ae3a8
 == refuse images that require too many offsets ===
9ae3a8
 qemu-io: can't open device TEST_DIR/simple-pattern.cloop: image requires too many offsets, try increasing block size
9ae3a8
 no file open, try 'help open'
9ae3a8
+
9ae3a8
+== refuse images with non-monotonically increasing offsets ==
9ae3a8
+qemu-io: can't open device TEST_DIR/simple-pattern.cloop: offsets not monotonically increasing at index 1, image file is corrupt
9ae3a8
+no file open, try 'help open'
9ae3a8
+
9ae3a8
+== refuse images with invalid compressed block size ==
9ae3a8
+qemu-io: can't open device TEST_DIR/simple-pattern.cloop: invalid compressed block size at index 1, image file is corrupt
9ae3a8
+no file open, try 'help open'
9ae3a8
 *** done
9ae3a8
-- 
9ae3a8
1.7.1
9ae3a8