[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    [oss-security] Xen Security Advisory 258 - Information leak via crafted user-supplied CDROM
From:       Xen.org security team <security () xen ! org>
Date:       2018-04-25 12:03:32
Message-ID: E1fBJ92-00043j-0a () xenbits ! xenproject ! org
[Download RAW message or body]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

                    Xen Security Advisory XSA-258
                              version 2

           Information leak via crafted user-supplied CDROM

UPDATES IN VERSION 2
====================

Public release.

ISSUE DESCRIPTION
=================

QEMU handles many different file formats for virtual disks (e.g., raw,
qcow2, vhd, &c).  Some of these formats are "snapshots" that specify
"patches" to an alternate disk image, whose filename is included in
the snapshot file.

When qemu is given a disk but the type is not specified, it attempts
to guess the file format by reading it.  If a disk image is intended
to be 'raw', but the image is entirely controlled by an attacker, the
attacker could write a header to the image, describing one of these
"snapshot" formats, and pointing to an arbitrary file as the "backing"
file.

When attaching disks via command-line parameters at boot time
(including both "normal" disks and CDROMs), libxl specifies the
format; however, when inserting a CDROM live via QMP, the format was
not specified.

IMPACT
======

An attacker supplying a crafted CDROM image can read any file (or
device node) on the dom0 filesystem with the permissions of the qemu
devicemodel process.  (The virtual CDROM device is read-only, so
no data can be written.)

VULNERABLE SYSTEMS
==================

Only x86 HVM guests with a virtual CDROM device are affected.  ARM
guests, x86 PV guests, x86 PVH guests, and x86 HVM guests without a
virtual CDROM device are not affected.

Only systems with qemu running in dom0 are affected; systems running
stub domains are not affected.  Only systems using qemu-xen (aka
"qemu-upstream" are affected; systems running qemu-xen-traditional
are not affected.

Only systems in which an attacker can provide a raw CDROM image, and
cause that image to be virtually inserted while the guest is running,
are affected.  Systems which only have host administrator-supplied
CDROM images, or systems which allow images to be added only at boot
time, are not affected.

MITIGATION
==========

One workaround is to "wrap" the guest-supplied image in a specific
format; i.e., accept a raw image from the untrusted user, and convert
it into qcow2 format; for example:

    qemu-img convert -f raw -O qcow2 untrusted.raw wrapped.qcow2

WARNING: Make sure to specify `-f raw` if you do this, or qemu will
"guess" the format of "untrusted.raw" (which the attacker may have
crafted to look like a qcow2 snapshot image with an alternativee base).

Another workaround is to allow guests to only change CDROMs at boot
time, not while the guest is running.

CREDITS
=======

This issue was discovered by Anthony Perard of Citrix.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa258.patch           xen-unstable, Xen 4.10.x, Xen 4.9.x
xsa258-4.8.patch       Xen 4.8.x, Xen 4.7.x
xsa258-4.6.patch       Xen 4.6.x

$ sha256sum xsa258*
2c35a77eeca5579b5c32517c5ba511c836fa70f8b824ca8883fc6e1a7e608405  xsa258.meta
7e8014deae4fa19464fe6570d0719f8f0d7730dd153d58b2fa38b0cd5ed2e459  xsa258.patch
2c58060a42dafbf65563941dd8c737732124b49eb47007cc60f647553227f557  xsa258-4.6.patch
ebba2f1f084249cd1e1c2f59e338412161884c31c83dbba03fc1e10bf4ba57a1  xsa258-4.8.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches and/or the "wrap" mitigation described above
(or others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

However, deploying the "only allow guests to change CDROMs at boot
time" is NOT permitted (except where all the affected systems and VMs
are administered and used only by organisations which are members of
the Xen Project Security Issues Predisclosure List).  Specifically,
deployment on public cloud systems is NOT permitted.  This is because
it may give attackers a hint of where to look for the vulnerability.
Deployment of this mitigation is permitted only AFTER the embargo
ends.

Additionally, distribution of updated software is prohibited (except
to other members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.

(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJa4G55AAoJEIP+FMlX6CvZHjYIAJEtdHT5yPyQuSjh8ATOYN/s
DrpUSw65EvvgbuGJTcmWZMc335AvyoMDtYVtk+Ouy5dMlfuUXcwjimoLWC6FfEDg
aJ19puvjVaA8JcRzimlWQjru8Eqyso1+uNjuvsv1RCSkhN6qGBGCx6xlyWJL0tGk
H/C9HPT7EAKw0bfyFJLOkl7PEohMxXSvGa9oiOZfEJnyr91AuvehTrQWM2Dwf2sz
sXp2drOlWQphwE3o/D+qDv5LOkyJY1NaKvGtTem3TmNT/YImMCWLZ3bS76GDE0io
qsMPHwndwMKDM7ST9bYcKy1Oq2f7DXHBcLVUtn1Q3DhPeSqBmxAfBuESveUIOl4=
=nyQb
-----END PGP SIGNATURE-----

["xsa258.meta" (application/octet-stream)]
["xsa258.patch" (application/octet-stream)]

From bf9ab0ec0b632739fe6366391e89a7d4dcf9993b Mon Sep 17 00:00:00 2001
From: Anthony PERARD <anthony.perard@citrix.com>
Date: Thu, 8 Mar 2018 18:16:41 +0000
Subject: [PATCH] libxl: Specify format of inserted cdrom

Without this extra parameter on the QMP command, QEMU will guess the
format of the new file.

This is XSA-258.

Reported-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl_device.c   | 13 +++++++++++++
 tools/libxl/libxl_dm.c       | 17 ++---------------
 tools/libxl/libxl_internal.h |  1 +
 tools/libxl/libxl_qmp.c      |  2 ++
 4 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c60cafe774..a4a8e9ac32 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -462,6 +462,19 @@ char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
     }
 }
 
+const char *libxl__qemu_disk_format_string(libxl_disk_format format)
+{
+    switch (format) {
+    case LIBXL_DISK_FORMAT_QCOW: return "qcow";
+    case LIBXL_DISK_FORMAT_QCOW2: return "qcow2";
+    case LIBXL_DISK_FORMAT_VHD: return "vpc";
+    case LIBXL_DISK_FORMAT_RAW: return "raw";
+    case LIBXL_DISK_FORMAT_EMPTY: return NULL;
+    case LIBXL_DISK_FORMAT_QED: return "qed";
+    default: return NULL;
+    }
+}
+
 int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor)
 {
     struct stat buf;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a3cddce8b7..b51178b9fd 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -677,19 +677,6 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
     return 0;
 }
 
-static const char *qemu_disk_format_string(libxl_disk_format format)
-{
-    switch (format) {
-    case LIBXL_DISK_FORMAT_QCOW: return "qcow";
-    case LIBXL_DISK_FORMAT_QCOW2: return "qcow2";
-    case LIBXL_DISK_FORMAT_VHD: return "vpc";
-    case LIBXL_DISK_FORMAT_RAW: return "raw";
-    case LIBXL_DISK_FORMAT_EMPTY: return NULL;
-    case LIBXL_DISK_FORMAT_QED: return "qed";
-    default: return NULL;
-    }
-}
-
 static char *dm_spice_options(libxl__gc *gc,
                                     const libxl_spice_info *spice)
 {
@@ -1516,9 +1503,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
              * always raw
              */
             if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK)
-                format = qemu_disk_format_string(disks[i].format);
+                format = libxl__qemu_disk_format_string(disks[i].format);
             else
-                format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
+                format = libxl__qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
 
             if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
                 if (!disks[i].is_cdrom) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 506687fbe9..0812be5376 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1202,6 +1202,7 @@ _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t,
 /* from xl_device */
 _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
 _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
+_hidden const char *libxl__qemu_disk_format_string(libxl_disk_format format);
 _hidden int libxl__device_disk_set_backend(libxl__gc*, libxl_device_disk*);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index eab993aca9..567ed1e772 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -982,6 +982,8 @@ int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid,
         return qmp_run_command(gc, domid, "eject", args, NULL, NULL);
     } else {
         qmp_parameters_add_string(gc, &args, "target", disk->pdev_path);
+        qmp_parameters_add_string(gc, &args, "arg",
+            libxl__qemu_disk_format_string(disk->format));
         return qmp_run_command(gc, domid, "change", args, NULL, NULL);
     }
 }
-- 
2.16.2


["xsa258-4.6.patch" (application/octet-stream)]

From 575bcd86ee0ce6d3082126415d371424fc7b5bdb Mon Sep 17 00:00:00 2001
From: Anthony PERARD <anthony.perard@citrix.com>
Date: Thu, 8 Mar 2018 18:16:41 +0000
Subject: [PATCH] libxl: Specify format of inserted cdrom

Without this extra parameter on the QMP command, QEMU will guess the
format of the new file.

This is XSA-258.

Reported-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl_device.c   | 12 ++++++++++++
 tools/libxl/libxl_dm.c       | 20 ++++++--------------
 tools/libxl/libxl_internal.h |  1 +
 tools/libxl/libxl_qmp.c      |  2 ++
 4 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index a81baee585..38ee43415f 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -395,6 +395,18 @@ char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
     }
 }
 
+const char *libxl__qemu_disk_format_string(libxl_disk_format format)
+{
+    switch (format) {
+    case LIBXL_DISK_FORMAT_QCOW: return "qcow";
+    case LIBXL_DISK_FORMAT_QCOW2: return "qcow2";
+    case LIBXL_DISK_FORMAT_VHD: return "vpc";
+    case LIBXL_DISK_FORMAT_RAW: return "raw";
+    case LIBXL_DISK_FORMAT_EMPTY: return NULL;
+    default: return NULL;
+    }
+}
+
 int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor)
 {
     struct stat buf;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0db5f13553..f238a8e4b2 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -656,18 +656,6 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
     return 0;
 }
 
-static const char *qemu_disk_format_string(libxl_disk_format format)
-{
-    switch (format) {
-    case LIBXL_DISK_FORMAT_QCOW: return "qcow";
-    case LIBXL_DISK_FORMAT_QCOW2: return "qcow2";
-    case LIBXL_DISK_FORMAT_VHD: return "vpc";
-    case LIBXL_DISK_FORMAT_RAW: return "raw";
-    case LIBXL_DISK_FORMAT_EMPTY: return NULL;
-    default: return NULL;
-    }
-}
-
 static char *dm_spice_options(libxl__gc *gc,
                                     const libxl_spice_info *spice)
 {
@@ -1115,7 +1103,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
             int disk, part;
             int dev_number =
                 libxl__device_disk_dev_number(disks[i].vdev, &disk, &part);
-            const char *format = qemu_disk_format_string(disks[i].format);
+            const char *format;
             char *drive;
             const char *pdev_path;
 
@@ -1125,6 +1113,11 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                 continue;
             }
 
+            if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK)
+                format = libxl__qemu_disk_format_string(disks[i].format);
+            else 
+                format = libxl__qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
+
             if (disks[i].is_cdrom) {
                 if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY)
                     drive = libxl__sprintf
@@ -1153,7 +1146,6 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                 }
 
                 if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
-                    format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
                     pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
                                                       disks[i].format);
                 } else {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a3597da22a..2e824f6249 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1136,6 +1136,7 @@ _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t,
 /* from xl_device */
 _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
 _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
+_hidden const char *libxl__qemu_disk_format_string(libxl_disk_format format);
 _hidden int libxl__device_disk_set_backend(libxl__gc*, libxl_device_disk*);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f798de74c5..3d52b87072 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -955,6 +955,8 @@ int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid,
         return qmp_run_command(gc, domid, "eject", args, NULL, NULL);
     } else {
         qmp_parameters_add_string(gc, &args, "target", disk->pdev_path);
+        qmp_parameters_add_string(gc, &args, "arg",
+            libxl__qemu_disk_format_string(disk->format));
         return qmp_run_command(gc, domid, "change", args, NULL, NULL);
     }
 }
-- 
2.16.2


["xsa258-4.8.patch" (application/octet-stream)]

From 437c3b3ad337c43056903e4824448428d3b5a956 Mon Sep 17 00:00:00 2001
From: Anthony PERARD <anthony.perard@citrix.com>
Date: Thu, 8 Mar 2018 18:16:41 +0000
Subject: [PATCH] libxl: Specify format of inserted cdrom

Without this extra parameter on the QMP command, QEMU will guess the
format of the new file.

This is XSA-258.

Reported-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl_device.c   | 12 ++++++++++++
 tools/libxl/libxl_dm.c       | 16 ++--------------
 tools/libxl/libxl_internal.h |  1 +
 tools/libxl/libxl_qmp.c      |  2 ++
 4 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 3e7a1026c4..cd2a980f18 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -425,6 +425,18 @@ char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
     }
 }
 
+const char *libxl__qemu_disk_format_string(libxl_disk_format format)
+{
+    switch (format) {
+    case LIBXL_DISK_FORMAT_QCOW: return "qcow";
+    case LIBXL_DISK_FORMAT_QCOW2: return "qcow2";
+    case LIBXL_DISK_FORMAT_VHD: return "vpc";
+    case LIBXL_DISK_FORMAT_RAW: return "raw";
+    case LIBXL_DISK_FORMAT_EMPTY: return NULL;
+    default: return NULL;
+    }
+}
+
 int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor)
 {
     struct stat buf;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index ad366a8cd3..b6bc407795 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -669,18 +669,6 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
     return 0;
 }
 
-static const char *qemu_disk_format_string(libxl_disk_format format)
-{
-    switch (format) {
-    case LIBXL_DISK_FORMAT_QCOW: return "qcow";
-    case LIBXL_DISK_FORMAT_QCOW2: return "qcow2";
-    case LIBXL_DISK_FORMAT_VHD: return "vpc";
-    case LIBXL_DISK_FORMAT_RAW: return "raw";
-    case LIBXL_DISK_FORMAT_EMPTY: return NULL;
-    default: return NULL;
-    }
-}
-
 static char *dm_spice_options(libxl__gc *gc,
                                     const libxl_spice_info *spice)
 {
@@ -1342,9 +1330,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
              * always raw
              */
             if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK)
-                format = qemu_disk_format_string(disks[i].format);
+                format = libxl__qemu_disk_format_string(disks[i].format);
             else
-                format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
+                format = libxl__qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
 
             if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
                 if (!disks[i].is_cdrom) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8366fee25f..c32a40576a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1170,6 +1170,7 @@ _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t,
 /* from xl_device */
 _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
 _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
+_hidden const char *libxl__qemu_disk_format_string(libxl_disk_format format);
 _hidden int libxl__device_disk_set_backend(libxl__gc*, libxl_device_disk*);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f8addf9ba6..6fc5454a6e 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -982,6 +982,8 @@ int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid,
         return qmp_run_command(gc, domid, "eject", args, NULL, NULL);
     } else {
         qmp_parameters_add_string(gc, &args, "target", disk->pdev_path);
+        qmp_parameters_add_string(gc, &args, "arg",
+            libxl__qemu_disk_format_string(disk->format));
         return qmp_run_command(gc, domid, "change", args, NULL, NULL);
     }
 }
-- 
2.16.2



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic