Commit 82ef1421 authored by Leonardo Arena's avatar Leonardo Arena

main/spice: security fix (CVE-2019-3813)

Fixes #9940
parent 8207e0e6
......@@ -2,7 +2,7 @@
# Maintainer: Natanael Copa <ncopa@alpinelinux.org>
pkgname=spice
pkgver=0.14.1
pkgrel=3
pkgrel=4
pkgdesc="Implements the SPICE protocol"
url="http://www.spice-space.org/"
arch="all"
......@@ -15,10 +15,13 @@ makedepends="$depends_dev alsa-lib-dev libjpeg-turbo-dev libxrandr-dev
subpackages="$pkgname-dev $pkgname-server"
source="https://www.spice-space.org/download/releases/spice-server/spice-$pkgver.tar.bz2
0001-Disable-failing-tests.patch
CVE-2019-3813.patch
"
builddir="$srcdir/$pkgname-$pkgver"
# secfixes:
# 0.14.1-r4:
# - CVE-2019-3813
# 0.14.1-r0:
# - CVE-2018-10873
# 0.12.8-r4:
......@@ -62,4 +65,5 @@ server() {
}
sha512sums="2c0b4fbcb68c76bc0404a807f28c9645a30c6b88e81d2bc574d63b036778a299cebc0ae12aa72f2e1496f66cbf414325125948d440541a40e1b9e53b8956542d spice-0.14.1.tar.bz2
7457d76ba056565de5b27d3fe0dd5969afbfc8e85a4f43345d491cdd79690eeb81c97d1012dba61562dcc240cac45a58ddb26d4a5ebdc71f4f5e191c5064f49f 0001-Disable-failing-tests.patch"
7457d76ba056565de5b27d3fe0dd5969afbfc8e85a4f43345d491cdd79690eeb81c97d1012dba61562dcc240cac45a58ddb26d4a5ebdc71f4f5e191c5064f49f 0001-Disable-failing-tests.patch
d64dd5ec03a18a1d1e5371595ad7d18055c607b54a7b381e0ad071fecf78abd8eac48a6152acaadec2ced90a9630a109f1af4caab0d0c7936b2c2642ac4dd107 CVE-2019-3813.patch"
From 6eff47e72cb2f23d168be58bab8bdd60df49afd0 Mon Sep 17 00:00:00 2001
From: Christophe Fergeau <cfergeau@redhat.com>
Date: Thu, 29 Nov 2018 14:18:39 +0100
Subject: [spice-server] memslot: Fix off-by-one error in group/slot boundary
check
RedMemSlotInfo keeps an array of groups, and each group contains an
array of slots. Unfortunately, these checks are off by 1, they check
that the index is greater or equal to the number of elements in the
array, while these arrays are 0 based. The check should only check for
strictly greater than the number of elements.
For the group array, this is not a big issue, as these memslot groups
are created by spice-server users (eg QEMU), and the group ids used to
index that array are also generated by the spice-server user, so it
should not be possible for the guest to set them to arbitrary values.
The slot id is more problematic, as it's calculated from a QXLPHYSICAL
address, and such addresses are usually set by the guest QXL driver, so
the guest can set these to arbitrary values, including malicious values,
which are probably easy to build from the guest PCI configuration.
This patch fixes the arrays bound check, and adds a test case for this.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
---
server/memslot.c | 4 ++--
server/tests/test-qxl-parsing.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/server/memslot.c b/server/memslot.c
index ede77e7..ea6f981 100644
--- a/server/memslot.c
+++ b/server/memslot.c
@@ -97,13 +97,13 @@ void *memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size
MemSlot *slot;
- if (group_id > info->num_memslots_groups) {
+ if (group_id >= info->num_memslots_groups) {
spice_critical("group_id too big");
return NULL;
}
slot_id = memslot_get_id(info, addr);
- if (slot_id > info->num_memslots) {
+ if (slot_id >= info->num_memslots) {
print_memslots(info);
spice_critical("slot_id %d too big, addr=%" PRIx64, slot_id, addr);
return NULL;
diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index 8565239f0..447425984 100644
--- a/server/tests/test-qxl-parsing.c
+++ b/server/tests/test-qxl-parsing.c
@@ -98,6 +98,31 @@ static void deinit_qxl_surface(QXLSurfaceCmd *qxl)
g_free(from_physical(qxl->u.surface_create.data));
}
+static void test_memslot_invalid_group_id(void)
+{
+ RedMemSlotInfo mem_info;
+ init_meminfo(&mem_info);
+
+ memslot_get_virt(&mem_info, 0, 16, 1);
+}
+
+static void test_memslot_invalid_slot_id(void)
+{
+ RedMemSlotInfo mem_info;
+ init_meminfo(&mem_info);
+
+ memslot_get_virt(&mem_info, 1 << mem_info.memslot_id_shift, 16, 0);
+}
+
+static void test_memslot_invalid_addresses(void)
+{
+ g_test_trap_subprocess("/server/memslot-invalid-addresses/subprocess/group_id", 0, 0);
+ g_test_trap_assert_stderr("*group_id too big*");
+
+ g_test_trap_subprocess("/server/memslot-invalid-addresses/subprocess/slot_id", 0, 0);
+ g_test_trap_assert_stderr("*slot_id 1 too big*");
+}
+
static void test_no_issues(void)
{
RedMemSlotInfo mem_info;
@@ -317,6 +342,11 @@ int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
+ /* try to use invalid memslot group/slot */
+ g_test_add_func("/server/memslot-invalid-addresses", test_memslot_invalid_addresses);
+ g_test_add_func("/server/memslot-invalid-addresses/subprocess/group_id", test_memslot_invalid_group_id);
+ g_test_add_func("/server/memslot-invalid-addresses/subprocess/slot_id", test_memslot_invalid_slot_id);
+
/* try to create a surface with no issues, should succeed */
g_test_add_func("/server/qxl-parsing-no-issues", test_no_issues);
--
2.19.2
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment