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

Fixes #9941

3 files changed, 171 insertions, 33 deletions

diff --git a/main/spice/0001-Disable-failing-tests.patch b/main/spice/0001-Disable-failing-tests-on-some-arches.patch
index b7b95d8a18a..21a081eaf7c 100644
--- a/main/spice/0001-Disable-failing-tests.patch
+++ b/main/spice/0001-Disable-failing-tests-on-some-arches.patch
@@ -1,7 +1,7 @@
-From 66f8bd209bdd8ed9c238c3e8641737eeecd70183 Mon Sep 17 00:00:00 2001
+From 5c306b874c847e6ae6750c55d097467ea89905b7 Mon Sep 17 00:00:00 2001
 From: Leonardo Arena <rnalrd@alpinelinux.org>
-Date: Wed, 7 Nov 2018 13:31:22 +0000
-Subject: [PATCH] Disable failing tests:  test-listem (x86_64), test-sasl (x86_64), test-leaks (x86), test-vdagent (ppc64le)
+Date: Thu, 31 Jan 2019 07:13:01 +0000
+Subject: [PATCH] Disable failing tests on some arches
 
 Missing logs for the last two tests
 
@@ -20,18 +20,21 @@ FAIL: test-sasl
 (process:27479): Spice-WARNING **: 10:54:41.853: red-stream.c:725:addr_to_string: Cannot resolve address -6: Unrecognized address family or invalid length
 **
 Spice:ERROR:test-sasl.c:516:client_emulator: assertion failed (read_u32_err(sock, &mechlen) == sizeof(uint32_t)): (0 == 4)
+
 ---
- server/tests/Makefile.am |  6 ---
- server/tests/Makefile.in | 98 +++-------------------------------------
- 2 files changed, 7 insertions(+), 97 deletions(-)
+ server/tests/Makefile.am |   7 ---
+ server/tests/Makefile.in | 122 +++------------------------------------
+ 2 files changed, 9 insertions(+), 120 deletions(-)
 
 diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
-index 238f25a..b09efcd 100644
+index 238f25a..51dbad0 100644
 --- a/server/tests/Makefile.am
 +++ b/server/tests/Makefile.am
-@@ -55,13 +55,10 @@ check_PROGRAMS =				\
+@@ -53,15 +53,11 @@ check_PROGRAMS =				\
+ 	test-stream				\
+ 	test-agent-msg-filter			\
  	test-loop				\
- 	test-qxl-parsing			\
+-	test-qxl-parsing			\
  	test-stat-file				\
 -	test-leaks				\
 -	test-vdagent				\
@@ -43,7 +46,7 @@ index 238f25a..b09efcd 100644
  	$(NULL)
  
  noinst_PROGRAMS =				\
-@@ -144,6 +141,3 @@ endif
+@@ -144,6 +140,3 @@ endif
  
  EXTRA_DIST += video-encoders
  
@@ -51,14 +54,16 @@ index 238f25a..b09efcd 100644
 -check_PROGRAMS += test-sasl
 -endif
 diff --git a/server/tests/Makefile.in b/server/tests/Makefile.in
-index bd2c74b..865c3c4 100644
+index bd2c74b..eeda989 100644
 --- a/server/tests/Makefile.in
 +++ b/server/tests/Makefile.in
-@@ -93,10 +93,9 @@ check_PROGRAMS = test-codecs-parsing$(EXEEXT) test-options$(EXEEXT) \
+@@ -92,11 +92,10 @@ host_triplet = @host@
+ check_PROGRAMS = test-codecs-parsing$(EXEEXT) test-options$(EXEEXT) \
  	test-stat$(EXEEXT) test-stream$(EXEEXT) \
  	test-agent-msg-filter$(EXEEXT) test-loop$(EXEEXT) \
- 	test-qxl-parsing$(EXEEXT) test-stat-file$(EXEEXT) \
+-	test-qxl-parsing$(EXEEXT) test-stat-file$(EXEEXT) \
 -	test-leaks$(EXEEXT) test-vdagent$(EXEEXT) \
++	test-stat-file$(EXEEXT) \
  	test-fail-on-null-core-interface$(EXEEXT) \
  	test-empty-success$(EXEEXT) test-channel$(EXEEXT) \
 -	test-stream-device$(EXEEXT) test-listen$(EXEEXT) \
@@ -105,10 +110,18 @@ index bd2c74b..865c3c4 100644
  test_loop_SOURCES = test-loop.c
  test_loop_OBJECTS = test-loop.$(OBJEXT)
  test_loop_LDADD = $(LDADD)
-@@ -311,14 +292,6 @@ test_qxl_parsing_DEPENDENCIES = libtest.a \
+@@ -303,22 +284,6 @@ test_playback_DEPENDENCIES = libtest.a \
  	$(top_builddir)/server/libserver.la $(am__DEPENDENCIES_1) \
  	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \
  	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1)
+-test_qxl_parsing_SOURCES = test-qxl-parsing.c
+-test_qxl_parsing_OBJECTS = test-qxl-parsing.$(OBJEXT)
+-test_qxl_parsing_LDADD = $(LDADD)
+-test_qxl_parsing_DEPENDENCIES = libtest.a \
+-	$(SPICE_COMMON_DIR)/common/libspice-common.la \
+-	$(top_builddir)/server/libserver.la $(am__DEPENDENCIES_1) \
+-	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \
+-	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1)
 -test_sasl_SOURCES = test-sasl.c
 -test_sasl_OBJECTS = test-sasl.$(OBJEXT)
 -test_sasl_LDADD = $(LDADD)
@@ -120,7 +133,7 @@ index bd2c74b..865c3c4 100644
  am_test_stat_OBJECTS = test-stat.$(OBJEXT)
  test_stat_OBJECTS = $(am_test_stat_OBJECTS)
  am__DEPENDENCIES_2 = libtest.a \
-@@ -361,14 +334,6 @@ test_two_servers_DEPENDENCIES = libtest.a \
+@@ -361,14 +326,6 @@ test_two_servers_DEPENDENCIES = libtest.a \
  	$(top_builddir)/server/libserver.la $(am__DEPENDENCIES_1) \
  	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \
  	$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1)
@@ -135,36 +148,37 @@ index bd2c74b..865c3c4 100644
  AM_V_P = $(am__v_P_@AM_V@)
  am__v_P_ = $(am__v_P_@AM_DEFAULT_V@)
  am__v_P_0 = false
-@@ -410,10 +375,10 @@ SOURCES = $(libtest_stat1_a_SOURCES) $(libtest_stat2_a_SOURCES) \
+@@ -410,10 +367,10 @@ SOURCES = $(libtest_stat1_a_SOURCES) $(libtest_stat2_a_SOURCES) \
  	test-display-no-ssl.c test-display-resolution-changes.c \
  	test-display-streaming.c test-display-width-stride.c \
  	test-empty-success.c test-fail-on-null-core-interface.c \
 -	$(test_gst_SOURCES) test-leaks.c test-listen.c test-loop.c \
 -	test-options.c test-playback.c test-qxl-parsing.c test-sasl.c \
 +	$(test_gst_SOURCES) test-loop.c \
-+	test-options.c test-playback.c test-qxl-parsing.c \
++	test-options.c test-playback.c \
  	$(test_stat_SOURCES) test-stat-file.c test-stream.c \
 -	test-stream-device.c test-two-servers.c test-vdagent.c
 +	test-stream-device.c test-two-servers.c
  DIST_SOURCES = $(libtest_stat1_a_SOURCES) $(libtest_stat2_a_SOURCES) \
  	$(libtest_stat3_a_SOURCES) $(libtest_stat4_a_SOURCES) \
  	$(libtest_a_SOURCES) $(spice_server_replay_SOURCES) \
-@@ -421,11 +386,10 @@ DIST_SOURCES = $(libtest_stat1_a_SOURCES) $(libtest_stat2_a_SOURCES) \
+@@ -421,11 +378,10 @@ DIST_SOURCES = $(libtest_stat1_a_SOURCES) $(libtest_stat2_a_SOURCES) \
  	test-display-no-ssl.c test-display-resolution-changes.c \
  	test-display-streaming.c test-display-width-stride.c \
  	test-empty-success.c test-fail-on-null-core-interface.c \
 -	$(am__test_gst_SOURCES_DIST) test-leaks.c test-listen.c \
-+	$(am__test_gst_SOURCES_DIST) \
- 	test-loop.c test-options.c test-playback.c test-qxl-parsing.c \
+-	test-loop.c test-options.c test-playback.c test-qxl-parsing.c \
 -	test-sasl.c $(test_stat_SOURCES) test-stat-file.c \
 -	test-stream.c test-stream-device.c test-two-servers.c \
 -	test-vdagent.c
++	$(am__test_gst_SOURCES_DIST) \
++	test-loop.c test-options.c test-playback.c \
 +	$(test_stat_SOURCES) test-stat-file.c \
 +	test-stream.c test-stream-device.c test-two-servers.c
  am__can_run_installinfo = \
    case $$AM_UPDATE_INFO_DIR in \
      n|no|NO) false;; \
-@@ -1046,14 +1010,6 @@ test-gst$(EXEEXT): $(test_gst_OBJECTS) $(test_gst_DEPENDENCIES) $(EXTRA_test_gst
+@@ -1046,14 +1002,6 @@ test-gst$(EXEEXT): $(test_gst_OBJECTS) $(test_gst_DEPENDENCIES) $(EXTRA_test_gst
  	@rm -f test-gst$(EXEEXT)
  	$(AM_V_CCLD)$(LINK) $(test_gst_OBJECTS) $(test_gst_LDADD) $(LIBS)
  
@@ -179,10 +193,14 @@ index bd2c74b..865c3c4 100644
  test-loop$(EXEEXT): $(test_loop_OBJECTS) $(test_loop_DEPENDENCIES) $(EXTRA_test_loop_DEPENDENCIES) 
  	@rm -f test-loop$(EXEEXT)
  	$(AM_V_CCLD)$(LINK) $(test_loop_OBJECTS) $(test_loop_LDADD) $(LIBS)
-@@ -1070,10 +1026,6 @@ test-qxl-parsing$(EXEEXT): $(test_qxl_parsing_OBJECTS) $(test_qxl_parsing_DEPEND
- 	@rm -f test-qxl-parsing$(EXEEXT)
- 	$(AM_V_CCLD)$(LINK) $(test_qxl_parsing_OBJECTS) $(test_qxl_parsing_LDADD) $(LIBS)
+@@ -1066,14 +1014,6 @@ test-playback$(EXEEXT): $(test_playback_OBJECTS) $(test_playback_DEPENDENCIES) $
+ 	@rm -f test-playback$(EXEEXT)
+ 	$(AM_V_CCLD)$(LINK) $(test_playback_OBJECTS) $(test_playback_LDADD) $(LIBS)
  
+-test-qxl-parsing$(EXEEXT): $(test_qxl_parsing_OBJECTS) $(test_qxl_parsing_DEPENDENCIES) $(EXTRA_test_qxl_parsing_DEPENDENCIES) 
+-	@rm -f test-qxl-parsing$(EXEEXT)
+-	$(AM_V_CCLD)$(LINK) $(test_qxl_parsing_OBJECTS) $(test_qxl_parsing_LDADD) $(LIBS)
+-
 -test-sasl$(EXEEXT): $(test_sasl_OBJECTS) $(test_sasl_DEPENDENCIES) $(EXTRA_test_sasl_DEPENDENCIES) 
 -	@rm -f test-sasl$(EXEEXT)
 -	$(AM_V_CCLD)$(LINK) $(test_sasl_OBJECTS) $(test_sasl_LDADD) $(LIBS)
@@ -190,7 +208,7 @@ index bd2c74b..865c3c4 100644
  test-stat$(EXEEXT): $(test_stat_OBJECTS) $(test_stat_DEPENDENCIES) $(EXTRA_test_stat_DEPENDENCIES) 
  	@rm -f test-stat$(EXEEXT)
  	$(AM_V_CCLD)$(LINK) $(test_stat_OBJECTS) $(test_stat_LDADD) $(LIBS)
-@@ -1094,10 +1046,6 @@ test-two-servers$(EXEEXT): $(test_two_servers_OBJECTS) $(test_two_servers_DEPEND
+@@ -1094,10 +1034,6 @@ test-two-servers$(EXEEXT): $(test_two_servers_OBJECTS) $(test_two_servers_DEPEND
  	@rm -f test-two-servers$(EXEEXT)
  	$(AM_V_CCLD)$(LINK) $(test_two_servers_OBJECTS) $(test_two_servers_LDADD) $(LIBS)
  
@@ -201,7 +219,7 @@ index bd2c74b..865c3c4 100644
  mostlyclean-compile:
  	-rm -f *.$(OBJEXT)
  	-rm -f ../*.$(OBJEXT)
-@@ -1123,19 +1071,15 @@ distclean-compile:
+@@ -1123,19 +1059,14 @@ distclean-compile:
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-empty-success.Po@am__quote@
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-fail-on-null-core-interface.Po@am__quote@
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-glib-compat.Po@am__quote@
@@ -210,7 +228,7 @@ index bd2c74b..865c3c4 100644
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-loop.Po@am__quote@
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-options.Po@am__quote@
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-playback.Po@am__quote@
- @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-qxl-parsing.Po@am__quote@
+-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-qxl-parsing.Po@am__quote@
 -@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-sasl.Po@am__quote@
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-stat-file.Po@am__quote@
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test-stat.Po@am__quote@
@@ -221,7 +239,21 @@ index bd2c74b..865c3c4 100644
  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/test_gst-test-gst.Po@am__quote@
  
  .c.o:
-@@ -1487,20 +1431,6 @@ test-stat-file.log: test-stat-file$(EXEEXT)
+@@ -1473,13 +1404,6 @@ test-loop.log: test-loop$(EXEEXT)
+ 	--log-file $$b.log --trs-file $$b.trs \
+ 	$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
+ 	"$$tst" $(AM_TESTS_FD_REDIRECT)
+-test-qxl-parsing.log: test-qxl-parsing$(EXEEXT)
+-	@p='test-qxl-parsing$(EXEEXT)'; \
+-	b='test-qxl-parsing'; \
+-	$(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
+-	--log-file $$b.log --trs-file $$b.trs \
+-	$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
+-	"$$tst" $(AM_TESTS_FD_REDIRECT)
+ test-stat-file.log: test-stat-file$(EXEEXT)
+ 	@p='test-stat-file$(EXEEXT)'; \
+ 	b='test-stat-file'; \
+@@ -1487,20 +1411,6 @@ test-stat-file.log: test-stat-file$(EXEEXT)
  	--log-file $$b.log --trs-file $$b.trs \
  	$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
  	"$$tst" $(AM_TESTS_FD_REDIRECT)
@@ -242,7 +274,7 @@ index bd2c74b..865c3c4 100644
  test-fail-on-null-core-interface.log: test-fail-on-null-core-interface$(EXEEXT)
  	@p='test-fail-on-null-core-interface$(EXEEXT)'; \
  	b='test-fail-on-null-core-interface'; \
-@@ -1529,20 +1459,6 @@ test-stream-device.log: test-stream-device$(EXEEXT)
+@@ -1529,20 +1439,6 @@ test-stream-device.log: test-stream-device$(EXEEXT)
  	--log-file $$b.log --trs-file $$b.trs \
  	$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
  	"$$tst" $(AM_TESTS_FD_REDIRECT)
@@ -264,5 +296,5 @@ index bd2c74b..865c3c4 100644
  	@p='video-encoders'; \
  	b='video-encoders'; \
 -- 
-2.19.1
+2.20.1
 
diff --git a/main/spice/APKBUILD b/main/spice/APKBUILD
index 4413dc2d0b9..43916ef8c90 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=0
+pkgrel=1
 pkgdesc="Implements the SPICE protocol"
 url="http://www.spice-space.org/"
 arch="all"
@@ -14,11 +14,14 @@ makedepends="$depends_dev alsa-lib-dev libjpeg-turbo-dev libxrandr-dev
 	py-six glib-dev opus-dev libressl-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
+	0001-Disable-failing-tests-on-some-arches.patch
+	CVE-2019-3813.patch
 	"
 builddir="$srcdir/$pkgname-$pkgver"
 
 # secfixes:
+#   0.14.1-r1:
+#     - 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"
+f53f538a3fda9b55395c57c9ddacbd43aff8de5214df5f61475db6234660cb2d27ff7de6b9631d5a2a840638d07bc732449c6ef60df030e08c423084406bb053  0001-Disable-failing-tests-on-some-arches.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 00000000000..1f80c1eb602
--- /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
+
+