Skip to content

Commit

Permalink
app-emulation/spice: fix vuln 0.13.1, bug #584126
Browse files Browse the repository at this point in the history
Apply the following patches to 0.13.1:

CVE-2016-2150:

  Commits 69628ea1375282cb7ca5b4dc4410e7aa67e0fc02
  Commits 790d8f3e53d324f496fc719498422e433aae8654

  *instead of* 0067-create-a-function-to-validate-surface-parameters.patch
  *instead of* 0068-improve-primary-surface-parameter-checks.patch

CVE-2016-0749:

  Ported the following commits to 0.13.1 (patches did not apply due to
  refactoring of some internal data structures and renaming).

  *modified* 0065-smartcard-add-a-ref-to-item-before-adding-to-pipe.patch
  *modified* 0066-smartcard-allocate-msg-with-the-expected-size.patch

Gentoo-Bug: 584126

Package-Manager: portage-2.2.28
  • Loading branch information
tamiko committed Jun 14, 2016
1 parent e78aee5 commit 76546db
Show file tree
Hide file tree
Showing 5 changed files with 343 additions and 0 deletions.
56 changes: 56 additions & 0 deletions app-emulation/spice/files/0.13.1-CVE-2016-0749-p1.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
From fd4a179a15882234f86ded87905a240dc76a9445 Mon Sep 17 00:00:00 2001
From: Matthias Maier <[email protected]>
Date: Tue, 14 Jun 2016 00:08:05 -0500
Subject: [PATCH 1/2] Port fix for CVE-2016-0749 to 0.13.1, part I

This is a port of

0065-smartcard-add-a-ref-to-item-before-adding-to-pipe.patch

to version 0.13.1

Original commit message:

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Marc-Andre Lureau <[email protected]>
Date: Thu, 17 Dec 2015 18:13:47 +0100
Subject: [PATCH] smartcard: add a ref to item before adding to pipe

There is an unref when the message is sent.

[...]

Signed-off-by: Marc-Andre Lureau <[email protected]>
---
server/smartcard.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/server/smartcard.c b/server/smartcard.c
index ba6f2f5..96e4295 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -181,14 +181,18 @@ static void smartcard_unref_msg_to_client(RedCharDeviceMsgToClient *msg,
smartcard_unref_vsc_msg_item((MsgItem *)msg);
}

-static void smartcard_send_msg_to_client(RedCharDeviceMsgToClient *msg,
+static void smartcard_send_msg_to_client(RedCharDeviceMsgToClient *message,
RedClient *client,
void *opaque)
{
RedCharDeviceSmartcard *dev = opaque;
- spice_assert(dev->priv->scc && dev->priv->scc->base.client == client);
- smartcard_channel_client_pipe_add_push(&dev->priv->scc->base, &((MsgItem *)msg)->base);

+ MsgItem *msg = (MsgItem *)message;
+ PipeItem *item = &msg->base;
+
+ spice_assert(dev->priv->scc && dev->priv->scc->base.client == client);
+ smartcard_ref_vsc_msg_item(msg);
+ smartcard_channel_client_pipe_add_push(&dev->priv->scc->base, item);
}

static void smartcard_send_tokens_to_client(RedClient *client, uint32_t tokens, void *opaque)
--
2.7.3

50 changes: 50 additions & 0 deletions app-emulation/spice/files/0.13.1-CVE-2016-0749-p2.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
From 4cd23b8378e68283c7c9cf0b1e25dae11cf69c3e Mon Sep 17 00:00:00 2001
From: Matthias Maier <[email protected]>
Date: Tue, 14 Jun 2016 00:15:48 -0500
Subject: [PATCH 2/2] Port fix for CVE-2016-0749 to 0.13.1, part II

This is a port of

0066-smartcard-allocate-msg-with-the-expected-size.patch

to version 0.13.1

Original commit message:

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Marc-Andre Lureau <[email protected]>
Date: Thu, 17 Dec 2015 18:16:22 +0100
Subject: [PATCH] smartcard: allocate msg with the expected size

[...]

Signed-off-by: Marc-Andre Lureau <[email protected]>
---
server/smartcard.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/server/smartcard.c b/server/smartcard.c
index 96e4295..c317512 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -313,7 +313,7 @@ static void smartcard_char_device_notify_reader_add(RedCharDeviceSmartcard *dev)
RedCharDeviceWriteBuffer *write_buf;
VSCMsgHeader *vheader;

- write_buf = red_char_device_write_buffer_get(RED_CHAR_DEVICE(dev), NULL, sizeof(vheader));
+ write_buf = red_char_device_write_buffer_get(RED_CHAR_DEVICE(dev), NULL, sizeof(*vheader));
if (!write_buf) {
spice_error("failed to allocate write buffer");
return;
@@ -360,7 +360,7 @@ static void smartcard_char_device_notify_reader_remove(RedCharDeviceSmartcard *d
spice_debug("reader add was never sent to the device");
return;
}
- write_buf = red_char_device_write_buffer_get(RED_CHAR_DEVICE(dev), NULL, sizeof(vheader));
+ write_buf = red_char_device_write_buffer_get(RED_CHAR_DEVICE(dev), NULL, sizeof(*vheader));
if (!write_buf) {
spice_error("failed to allocate write buffer");
return;
--
2.7.3

109 changes: 109 additions & 0 deletions app-emulation/spice/files/0.13.1-CVE-2016-2150-p1.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
From fc06265c3780e05503410a6646d1434e15d25b03 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <[email protected]>
Date: Mon, 29 Feb 2016 14:24:03 +0000
Subject: [PATCH 1/2] factor out red_validate_surface function to validate
surface parameters

Make possible to reuse it outside red-parse-qxl.c.

Signed-off-by: Frediano Ziglio <[email protected]>
Acked-by: Christophe Fergeau <[email protected]>
---
server/red-parse-qxl.c | 49 ++++++++++++++++++++++++++++++++-----------------
server/red-parse-qxl.h | 3 +++
2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 18b7ea6..b462311 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1327,13 +1327,41 @@ static unsigned int surface_format_to_bpp(uint32_t format)
return 0;
}

+bool red_validate_surface(uint32_t width, uint32_t height,
+ int32_t stride, uint32_t format)
+{
+ unsigned int bpp;
+ uint64_t size;
+
+ bpp = surface_format_to_bpp(format);
+
+ /* check if format is valid */
+ if (!bpp) {
+ return false;
+ }
+
+ /* check stride is larger than required bytes */
+ size = ((uint64_t) width * bpp + 7u) / 8u;
+ /* the uint32_t conversion is here to avoid problems with -2^31 value */
+ if (stride == G_MININT32 || size > (uint32_t) abs(stride)) {
+ return false;
+ }
+
+ /* the multiplication can overflow, also abs(-2^31) may return a negative value */
+ size = (uint64_t) height * abs(stride);
+ if (size > MAX_DATA_CHUNK) {
+ return false;
+ }
+
+ return true;
+}
+
int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
RedSurfaceCmd *red, QXLPHYSICAL addr)
{
QXLSurfaceCmd *qxl;
uint64_t size;
int error;
- unsigned int bpp;

qxl = (QXLSurfaceCmd *)memslot_get_virt(slots, addr, sizeof(*qxl), group_id,
&error);
@@ -1353,26 +1381,13 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
red->u.surface_create.width = qxl->u.surface_create.width;
red->u.surface_create.height = qxl->u.surface_create.height;
red->u.surface_create.stride = qxl->u.surface_create.stride;
- bpp = surface_format_to_bpp(red->u.surface_create.format);

- /* check if format is valid */
- if (!bpp) {
+ if (!red_validate_surface(red->u.surface_create.width, red->u.surface_create.height,
+ red->u.surface_create.stride, red->u.surface_create.format)) {
return 1;
}

- /* check stride is larger than required bytes */
- size = ((uint64_t) red->u.surface_create.width * bpp + 7u) / 8u;
- /* the uint32_t conversion is here to avoid problems with -2^31 value */
- if (red->u.surface_create.stride == G_MININT32
- || size > (uint32_t) abs(red->u.surface_create.stride)) {
- return 1;
- }
-
- /* the multiplication can overflow, also abs(-2^31) may return a negative value */
- size = (uint64_t) red->u.surface_create.height * abs(red->u.surface_create.stride);
- if (size > MAX_DATA_CHUNK) {
- return 1;
- }
+ size = red->u.surface_create.height * abs(red->u.surface_create.stride);
red->u.surface_create.data =
(uint8_t*)memslot_get_virt(slots, qxl->u.surface_create.data, size, group_id, &error);
if (error) {
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 9c30572..72a57b4 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -127,6 +127,9 @@ int red_get_message(RedMemSlotInfo *slots, int group_id,
RedMessage *red, QXLPHYSICAL addr);
void red_put_message(RedMessage *red);

+bool red_validate_surface(uint32_t width, uint32_t height,
+ int32_t stride, uint32_t format);
+
int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
RedSurfaceCmd *red, QXLPHYSICAL addr);
void red_put_surface_cmd(RedSurfaceCmd *red);
--
2.7.3

50 changes: 50 additions & 0 deletions app-emulation/spice/files/0.13.1-CVE-2016-2150-p2.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
From b1c167bb9e8047e93bfd43a43832963c8e830f5b Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <[email protected]>
Date: Wed, 2 Mar 2016 12:35:41 +0000
Subject: [PATCH 2/2] improve primary surface parameter checks

Primary surface, as additional surfaces, can be used to access
host memory from the guest using invalid parameters.

The removed warning is not enough to prevent all cases. Also a warning
is not enough to stop an escalation to happen.
The red_validate_surface do different checks to make sure surface
request is valid and not cause possible buffer/integer overflows:
- format is valid;
- width is not large to cause overflow compared to stride;
- stride is not -2^31 (a number which negate is still <0);
- stride * height does not overflow.

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1312980.

Signed-off-by: Frediano Ziglio <[email protected]>
Acked-by: Christophe Fergeau <[email protected]>
---
server/red-worker.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/server/red-worker.c b/server/red-worker.c
index 241c300..c7fc8bd 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -681,8 +681,15 @@ static void dev_create_primary_surface(RedWorker *worker, uint32_t surface_id,
spice_debug(NULL);
spice_warn_if_fail(surface_id == 0);
spice_warn_if_fail(surface.height != 0);
- spice_warn_if_fail(((uint64_t)abs(surface.stride) * (uint64_t)surface.height) ==
- abs(surface.stride) * surface.height);
+
+ /* surface can arrive from guest unchecked so make sure
+ * guest is not a malicious one and drop invalid requests
+ */
+ if (!red_validate_surface(surface.width, surface.height,
+ surface.stride, surface.format)) {
+ spice_warning("wrong primary surface creation request");
+ return;
+ }

line_0 = (uint8_t*)memslot_get_virt(&worker->mem_slots, surface.mem,
surface.height * abs(surface.stride),
--
2.7.3

78 changes: 78 additions & 0 deletions app-emulation/spice/spice-0.13.1-r2.ebuild
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Copyright 1999-2016 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Id$

EAPI=5

PYTHON_COMPAT=( python{2_7,3_4} )

inherit eutils python-any-r1

DESCRIPTION="SPICE server"
HOMEPAGE="http://spice-space.org/"
SRC_URI="http://spice-space.org/download/releases/${P}.tar.bz2"

LICENSE="LGPL-2.1"
SLOT="0"
KEYWORDS="~amd64 ~x86"
IUSE="libressl lz4 sasl smartcard static-libs"

# the libspice-server only uses the headers of libcacard
RDEPEND="
>=dev-libs/glib-2.22:2[static-libs(+)?]
>=media-libs/celt-0.5.1.1:0.5.1[static-libs(+)?]
media-libs/opus[static-libs(+)?]
sys-libs/zlib[static-libs(+)?]
virtual/jpeg:0=[static-libs(+)?]
>=x11-libs/pixman-0.17.7[static-libs(+)?]
!libressl? ( dev-libs/openssl:0[static-libs(+)?] )
libressl? ( dev-libs/libressl[static-libs(+)?] )
lz4? ( app-arch/lz4 )
smartcard? ( >=app-emulation/libcacard-0.1.2 )
sasl? ( dev-libs/cyrus-sasl[static-libs(+)?] )"

DEPEND="
~app-emulation/spice-protocol-0.12.11
virtual/pkgconfig
$(python_gen_any_dep '
>=dev-python/pyparsing-1.5.6-r2[${PYTHON_USEDEP}]
dev-python/six[${PYTHON_USEDEP}]
')
smartcard? ( app-emulation/qemu[smartcard] )
${RDEPEND}"

python_check_deps() {
has_version ">=dev-python/pyparsing-1.5.6-r2[${PYTHON_USEDEP}]"
has_version "dev-python/six[${PYTHON_USEDEP}]"
}

pkg_setup() {
[[ ${MERGE_TYPE} != binary ]] && python-any-r1_pkg_setup
}

# maintainer notes:
# * opengl support is currently broken

src_prepare() {
epatch \
"${FILESDIR}"/${PV}-CVE-2016-0749-p1.patch \
"${FILESDIR}"/${PV}-CVE-2016-0749-p2.patch \
"${FILESDIR}"/${PV}-CVE-2016-2150-p1.patch \
"${FILESDIR}"/${PV}-CVE-2016-2150-p2.patch

epatch_user
}

src_configure() {
econf \
$(use_enable static-libs static) \
$(use_enable lz4) \
$(use_with sasl) \
$(use_enable smartcard) \
--disable-gui
}

src_install() {
default
use static-libs || prune_libtool_files
}

0 comments on commit 76546db

Please sign in to comment.