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

2 files changed, 108 insertions, 2 deletions

diff --git a/main/spice/APKBUILD b/main/spice/APKBUILD
index d098ac1f14..072aa699a8 100644
--- a/main/spice/APKBUILD
+++ b/main/spice/APKBUILD
@@ -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"
diff --git a/main/spice/CVE-2019-3813.patch b/main/spice/CVE-2019-3813.patch
new file mode 100644
index 0000000000..1f80c1eb60
--- /dev/null
+++ b/main/spice/CVE-2019-3813.patch
@@ -0,0 +1,102 @@
+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
+
+