Bug 445844 - (CVE-2008-1945) CVE-2008-1945 qemu/kvm/xen: add image format options for USB storage and removable media
CVE-2008-1945 qemu/kvm/xen: add image format options for USB storage and remo...
Status: CLOSED CURRENTRELEASE
Product: Security Response
Classification: Other
Component: vulnerability (Show other bugs)
unspecified
All Linux
high Severity high
: ---
: ---
Assigned To: Red Hat Product Security
reported=20080508,public=20080807,sou...
: Security
Depends On: 445845 445846
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-09 07:15 EDT by Jan Lieskovsky
Modified: 2009-10-22 11:56 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-10-22 11:56:12 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jan Lieskovsky 2008-05-09 07:15:49 EDT
Description of problem:

Chris Wright has reported the following qemu removable (usb, floppy) media
related problem (pointed out by Markus Armbruster):

Previous commit didn't handle removable media or USB (thanks to Markus
for noting this).  This patch adds a cmdline option for USB to allow
admin to specify format type.  To avoid changing exists semantics a new
option -usbdevice diskformat: is added (ugly name). This is valid from
both command line and monitor interface.  Because of the comma delimiter,
admin must use ',,' just as in -drive file=filename.

The patch also allows specifying image format when changing removable
media.  It is an optional argument to the monitor command "change,"
so there is no change to existing semantics.

Longer term it'd be better to provide some safe defaults.

Additional info:

This issue is related with qemu block format auto-detection vulnerability
(CVE-2008-2004).
Comment 1 Jan Lieskovsky 2008-05-09 07:17:29 EDT
Proposed patch from Chris Wright:

---
 hw/usb-msd.c  |   12 ++++++++++--
 hw/usb.h      |    2 +-
 monitor.c     |   20 ++++++++++++++------
 qemu-doc.texi |    4 ++++
 vl.c          |   13 ++++++++++++-
 5 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 01d492d..3c3f39a 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -513,17 +513,25 @@ static void usb_msd_handle_destroy(USBDevice *dev)
     qemu_free(s);
 }
 
-USBDevice *usb_msd_init(const char *filename)
+USBDevice *usb_msd_init(const char *filename, const char *fmt)
 {
     MSDState *s;
     BlockDriverState *bdrv;
+    BlockDriver *drv = NULL;
 
+    if (fmt) {
+        drv = bdrv_find_format(fmt);
+        if (!drv) {
+            fprintf(stderr, "%s: '%s' invalid format\n", __func__, fmt);
+            return NULL;
+        }
+    }
     s = qemu_mallocz(sizeof(MSDState));
     if (!s)
         return NULL;
 
     bdrv = bdrv_new("usb");
-    if (bdrv_open(bdrv, filename, 0) < 0)
+    if (bdrv_open2(bdrv, filename, 0, drv) < 0)
         goto fail;
     if (qemu_key_check(bdrv, filename))
         goto fail;
diff --git a/hw/usb.h b/hw/usb.h
index 9b759d4..a2cb313 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -217,7 +217,7 @@ USBDevice *usb_tablet_init(void);
 USBDevice *usb_keyboard_init(void);
 
 /* usb-msd.c */
-USBDevice *usb_msd_init(const char *filename);
+USBDevice *usb_msd_init(const char *filename, const char *fmt);
 
 /* usb-wacom.c */
 USBDevice *usb_wacom_init(void);
diff --git a/monitor.c b/monitor.c
index 236b827..8d79d5c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -405,18 +405,26 @@ static void do_eject(int force, const char *filename)
     eject_device(bs, force);
 }
 
-static void do_change_block(const char *device, const char *filename)
+static void do_change_block(const char *device, const char *filename, const
char *fmt)
 {
     BlockDriverState *bs;
+    BlockDriver *drv = NULL;
 
     bs = bdrv_find(device);
     if (!bs) {
         term_printf("device not found\n");
         return;
     }
+    if (fmt) {
+        drv = bdrv_find_format(fmt);
+        if (!drv) {
+            term_printf("invalid format %s\n", fmt);
+            return;
+        }
+    }
     if (eject_device(bs, 0) < 0)
         return;
-    bdrv_open(bs, filename, 0);
+    bdrv_open2(bs, filename, 0, drv);
     qemu_key_check(bs, filename);
 }
 
@@ -435,12 +443,12 @@ static void do_change_vnc(const char *target)
     }
 }
 
-static void do_change(const char *device, const char *target)
+static void do_change(const char *device, const char *target, const char *fmt)
 {
     if (strcmp(device, "vnc") == 0) {
        do_change_vnc(target);
     } else {
-       do_change_block(device, target);
+       do_change_block(device, target, fmt);
     }
 }
 
@@ -1322,8 +1330,8 @@ static term_cmd_t term_cmds[] = {
       "", "quit the emulator" },
     { "eject", "-fB", do_eject,
       "[-f] device", "eject a removable medium (use -f to force it)" },
-    { "change", "BF", do_change,
-      "device filename", "change a removable medium" },
+    { "change", "BFs?", do_change,
+      "device filename [format]", "change a removable medium, optional format" },
     { "screendump", "F", do_screen_dump,
       "filename", "save screen into PPM image 'filename'" },
     { "logfile", "s", do_logfile,
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 4a48f43..34743f2 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -547,6 +547,10 @@ mouse. Also overrides the PS/2 mouse emulation when activated.
 @item disk:file
 Mass storage device based on file
 
+@item diskformat:file,format=@var{format}
+Mass storage device based on file with specified image @var{format}.
+See -drive file= and format= for more information.
+
 @item host:bus.addr
 Pass through the host device identified by bus.addr (Linux only).
 
diff --git a/vl.c b/vl.c
index ea8c63c..febecef 100644
--- a/vl.c
+++ b/vl.c
@@ -5351,7 +5351,18 @@ static int usb_device_add(const char *devname)
     } else if (!strcmp(devname, "keyboard")) {
         dev = usb_keyboard_init();
     } else if (strstart(devname, "disk:", &p)) {
-        dev = usb_msd_init(p);
+        dev = usb_msd_init(p, NULL);
+    } else if (strstart(devname, "diskformat:", &p)) {
+        char file[1024];
+        char buf[128];
+        char *fmt = NULL;
+        p = get_opt_value(file, sizeof(file), p);
+        if (*p == ',') {
+            p++;
+            if (get_param_value(buf, sizeof(buf), "format", p))
+                fmt = buf;
+        }
+        dev = usb_msd_init(file, fmt);
     } else if (!strcmp(devname, "wacom-tablet")) {
         dev = usb_wacom_init();
     } else if (strstart(devname, "serial:", &p)) {
Comment 2 Jan Lieskovsky 2008-05-09 07:18:23 EDT
Yet more details from Chris:

Unfortunately, this doesn't care for removable media.

(qemu) info block
floppy0: type=floppy removable=1 locked=0 file=/mnt/disk2 ro=0 drv=raw

guest writes malicious qcow header to its /dev/floppy

(qemu) change floppy0 /mnt/disk2
(qemu) info block
floppy0: type=floppy removable=1 locked=0 file=/mnt/disk2
backing_file=/etc/passwd ro=0 drv=qcow2

Nor does it care for usb disk devices.  I've got a patch for these,
although it's getting a bit ugly.  Below is against qemu svn tip.
Comment 5 Markus Armbruster 2008-05-09 10:23:56 EDT
There's no need to fix QEMU monitor commands for RHEL-5, because we disabled the
monitor (bug 230295).

We need to fix -usbdevice, because that is accessible through /etc/xen/DOMNAME
key usbdevice.
Comment 6 Tomas Hoger 2008-09-09 04:49:03 EDT
Issue is public already, opening bug.
Comment 7 Chris Lalancette 2009-10-22 11:56:12 EDT
The dependant bugs were closed out long ago, closing out this tracker.

Chris Lalancette

Note You need to log in before you can comment on or make changes to this bug.