Xen Security Advisory 335 v2 (CVE-2020-14364) - QEMU: usb: out-of-bounds r/w access issue

Xen Security Advisory 335 v2 (CVE-2020-14364) - QEMU: usb: out-of-bounds r/w access issue

[Xen.org security team]

[-- Attachment #1: Type: text/plain, Size: 4175 bytes --]

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

            Xen Security Advisory CVE-2020-14364 / XSA-335
                               version 2

               QEMU: usb: out-of-bounds r/w access issue

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

Don't break the DSO by eliding the SoB on the patch.

Update Vulnerable Systems section.

Public release.

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

An out-of-bounds read/write access issue was found in the USB emulator
of the QEMU. It occurs while processing USB packets from a guest, when
'USBDevice->setup_len' exceeds the USBDevice->data_buf[4096], in
do_token_{in,out} routines.

IMPACT
======

A guest user may use this flaw to crash the QEMU process resulting in
DoS OR potentially execute arbitrary code with the privileges of the
QEMU process on the host.

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

All versions of Qemu shipped with in-support versions of Xen are
vulnerable.  This includes both qemu-traditional and qemu-xen.

The vulnerability can only be exploited when Qemu is used as a device
model.  This configuration is only used by default for x86 HVM guests.
x86 PV, PVH and ARM guest do not use a device model by default.

Guests configured to use a Qemu stubdomain contain the code execution
within the stubdomain, and are therefore not considered vulnerable.

MITIGATION
==========

No mitigation is available.

CREDITS
=======

This issue was discovered by Xiao Wei of Qihoo 360 Inc.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball.  Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.

xsa335-qemu.patch    QEMU
xsa335-trad.patch    Xen unstable (SUPPORT.md update only)

$ sha256sum xsa335*
3af5f30c4fd21e3679fb749659f9e59d0ff335d092254352e128e7fee3340c41  xsa335-qemu.patch
2ed7b8bac4c473c6f89173a73485904be16785eb29ee18e189717d201381f27f  xsa335-trad.patch
$

"QEMU XEN TRADITIONAL"
======================

This version of qemu is provided by the Xen Project for use as a
device model stub domain.  In that configuration, there is not a
security problem and no action is needed.

But in other configurations, this version of qemu is lacking many
security fixes.  It is beyond the capacity of the Xen Project Security
Team to address these.  There is therefore no code resolution to
XSA-335 for users of qemu-xen-traditional who are not using device
model stub domains.

The patch xsa335-trad.patch included in this advisory is merely an
update for Xen's SUPPORT.md to document this situation.

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

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

But: 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-----

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAl9Dr+0MHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZ274H/3FIK/DecsmdqVFs9UjqCi+RABmz6dFsgUxQYH9c
ysZvN7R/BTR1m425+7tlPK1oglkFkHt6C9snc3+kTh/Bl5ktXakgVacoR6yeTh88
1yJQC3JmG9OaXGS4AR9hmE+Wg0XTlrmvzPMFxtWv055kpPVEG6FWhnhV8d0FavoI
RWnlelNSkXgai5zWlAqhF8jzR4EeEmOp4f/BtQX/cjZAodXZSYMvLW1zy3vx4Wik
ZpL4qkJLE9GHOYZF9Ng8zwWx7c1CIi76zwdUvUgPu6IjTBIpo0LPZxlkbF+CqYcp
rVFaAy7j7+xMOOJntlN2a/NAxD4zs+sCLF1legrfi+9uMH4=
=bMZs
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa335-qemu.patch --]
[-- Type: application/octet-stream, Size: 3042 bytes --]

From c5bd2924c6d6a5bcbffb8b5e7798a88970131c07 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 17 Aug 2020 08:34:22 +0200
Subject: [PATCH] usb: fix setup_len init (CVE-2020-14364)

Store calculated setup_len in a local variable, verify it, and only
write it to the struct (USBDevice->setup_len) in case it passed the
sanity checks.

This prevents other code (do_token_{in,out} functions specifically)
from working with invalid USBDevice->setup_len values and overrunning
the USBDevice->setup_buf[] buffer.

Fixes: CVE-2020-14364
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/core.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/hw/usb/core.c b/hw/usb/core.c
index 5abd128b6bc5..5234dcc73fea 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -129,6 +129,7 @@ void usb_wakeup(USBEndpoint *ep, unsigned int stream)
 static void do_token_setup(USBDevice *s, USBPacket *p)
 {
     int request, value, index;
+    unsigned int setup_len;
 
     if (p->iov.size != 8) {
         p->status = USB_RET_STALL;
@@ -138,14 +139,15 @@ static void do_token_setup(USBDevice *s, USBPacket *p)
     usb_packet_copy(p, s->setup_buf, p->iov.size);
     s->setup_index = 0;
     p->actual_length = 0;
-    s->setup_len   = (s->setup_buf[7] << 8) | s->setup_buf[6];
-    if (s->setup_len > sizeof(s->data_buf)) {
+    setup_len = (s->setup_buf[7] << 8) | s->setup_buf[6];
+    if (setup_len > sizeof(s->data_buf)) {
         fprintf(stderr,
                 "usb_generic_handle_packet: ctrl buffer too small (%d > %zu)\n",
-                s->setup_len, sizeof(s->data_buf));
+                setup_len, sizeof(s->data_buf));
         p->status = USB_RET_STALL;
         return;
     }
+    s->setup_len = setup_len;
 
     request = (s->setup_buf[0] << 8) | s->setup_buf[1];
     value   = (s->setup_buf[3] << 8) | s->setup_buf[2];
@@ -259,26 +261,28 @@ static void do_token_out(USBDevice *s, USBPacket *p)
 static void do_parameter(USBDevice *s, USBPacket *p)
 {
     int i, request, value, index;
+    unsigned int setup_len;
 
     for (i = 0; i < 8; i++) {
         s->setup_buf[i] = p->parameter >> (i*8);
     }
 
     s->setup_state = SETUP_STATE_PARAM;
-    s->setup_len   = (s->setup_buf[7] << 8) | s->setup_buf[6];
     s->setup_index = 0;
 
     request = (s->setup_buf[0] << 8) | s->setup_buf[1];
     value   = (s->setup_buf[3] << 8) | s->setup_buf[2];
     index   = (s->setup_buf[5] << 8) | s->setup_buf[4];
 
-    if (s->setup_len > sizeof(s->data_buf)) {
+    setup_len = (s->setup_buf[7] << 8) | s->setup_buf[6];
+    if (setup_len > sizeof(s->data_buf)) {
         fprintf(stderr,
                 "usb_generic_handle_packet: ctrl buffer too small (%d > %zu)\n",
-                s->setup_len, sizeof(s->data_buf));
+                setup_len, sizeof(s->data_buf));
         p->status = USB_RET_STALL;
         return;
     }
+    s->setup_len = setup_len;
 
     if (p->pid == USB_TOKEN_OUT) {
         usb_packet_copy(p, s->data_buf, s->setup_len);
-- 
2.18.4

[-- Attachment #3: xsa335-trad.patch --]
[-- Type: application/octet-stream, Size: 1647 bytes --]

From a62cdd675bc6a8053f6797b6add29b2853b081e3 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 19 Aug 2020 18:31:45 +0100
Subject: [PATCH] SUPPORT.md: Desupport qemu trad except stub dm

While investigating XSA-335 we discovered that many upstream security
fixes were missing.  It is not practical to backport them.  There is
no good reason to be running this very ancient version of qemu, except
that it is the only way to run a stub dm which is currently supported
by upstream.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 SUPPORT.md | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/SUPPORT.md b/SUPPORT.md
index 1479055c45..b0939052e2 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -758,7 +758,21 @@ See the section **Blkback** for image formats supported by QEMU.
 
     Status: Supported, not security supported
 
+### qemu-xen-traditional ###
+
+The Xen Project provides an old version of qemu with modifications
+which enable use as a device model stub domain.  The old version is
+normally selected by default only in a stub dm configuration, but it
+can be requested explicitly in other configurations, for example in
+`xl` with `device_model_version="QEMU_XEN_TRADITIONAL"`.
+
+    Status, Device Model Stub Domains: Supported, with caveats
+    Status, as host process device model: No security support, not recommended
+
+qemu-xen-traditional is security supported only for those available
+devices which are supported for mainstream QEMU (see above), with
+trusted driver domains (see Device Model Stub Domains).
+
 ## Virtual Firmware
 
 ### x86/HVM iPXE
-- 
2.20.1