[pve-devel] [PATCH libspice-server] update spice to 0.14.3 and add patches for CVE-2020-14355

[pve-devel] [PATCH libspice-server] update spice to 0.14.3 and add patches for CVE-2020-14355

[Oguz Bektas]

the original email is in oss-security[0], patches are from upstream
gitlab. (commits linked in the email)

removed the already applied monitor patch.

[0]: https://www.openwall.com/lists/oss-security/2020/10/06/10

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 Makefile                                      | 10 ++---
 changelog.Debian                              |  7 +++
 patches/CVE-2020-14355_1.patch                | 28 ++++++++++++
 patches/CVE-2020-14355_2.patch                | 43 +++++++++++++++++++
 patches/CVE-2020-14355_3.patch                | 30 +++++++++++++
 patches/CVE-2020-14355_4.patch                | 29 +++++++++++++
 ...ke-sure-we-have-at-least-one-monitor.patch | 33 --------------
 7 files changed, 142 insertions(+), 38 deletions(-)
 create mode 100644 patches/CVE-2020-14355_1.patch
 create mode 100644 patches/CVE-2020-14355_2.patch
 create mode 100644 patches/CVE-2020-14355_3.patch
 create mode 100644 patches/CVE-2020-14355_4.patch
 delete mode 100644 patches/red-qxl-make-sure-we-have-at-least-one-monitor.patch

diff --git a/Makefile b/Makefile
index 01bed58..6a34c84 100644
--- a/Makefile
+++ b/Makefile
@@ -1,9 +1,9 @@
 SOURCE=spice
 PACKAGE=libspice-server1
 
-PKGVERSION=0.14.2
-DEBVERSION=0.14.2-4
-PVERELEASE=pve6+1
+PKGVERSION=0.14.3
+DEBVERSION=0.14.3-1
+PVERELEASE=pve6
 
 VERSION := $(DEBVERSION)~$(PVERELEASE)
 
@@ -29,7 +29,7 @@ $(DEB): $(SOURCE)_$(PKGVERSION).orig.tar.bz2 $(SOURCE)_$(DEBVERSION).debian.tar.
 	tar xf $(SOURCE)_$(DEBVERSION).debian.tar.xz -C $(SOURCE)-$(PKGVERSION)
 	cat changelog.Debian $(PKGDIR)/debian/changelog > $(PKGDIR)/debian/changelog.tmp
 	mv $(PKGDIR)/debian/changelog.tmp $(PKGDIR)/debian/changelog
-	cd $(PKGDIR); for patch in ../patches/*.patch; do echo "applying patch '$$patch'" && patch -p1 < "$${patch}"; done
+	cd $(PKGDIR); for patch in ../patches/*.patch; do echo "applying patch '$$patch'" && patch -Np1 < "$${patch}"; done
 	cd ${PKGDIR}; dpkg-buildpackage -b -us -uc
 	lintian ${DEBS}
 
@@ -38,7 +38,7 @@ $(DEB): $(SOURCE)_$(PKGVERSION).orig.tar.bz2 $(SOURCE)_$(DEBVERSION).debian.tar.
 download: $(SOURCE)_$(PKGVERSION).orig.tar.bz2 $(SOURCE)_$(DEBVERSION).debian.tar.xz
 $(SOURCE)_$(PKGVERSION).orig.tar.bz2: $(SOURCE)_$(DEBVERSION).debian.tar.xz
 $(SOURCE)_$(DEBVERSION).debian.tar.xz:
-	dget http://deb.debian.org/debian/pool/main/s/spice/spice_0.14.2-4.dsc
+	dget http://deb.debian.org/debian/pool/main/s/spice/spice_0.14.3-1.dsc
 
 .PHONY: upload
 upload: ${DEBS}
diff --git a/changelog.Debian b/changelog.Debian
index 14c019c..d1b4489 100644
--- a/changelog.Debian
+++ b/changelog.Debian
@@ -1,3 +1,10 @@
+spice (0.14.3-1~pve6) pve; urgency=medium
+
+  [ Proxmox Support Team ]
+  * add patches for CVE-2020-14355 from oss-security
+
+ -- Proxmox Support Team <support@proxmox.com>  Tue, 06 Oct 2020 16:33:30 +0200
+
 spice (0.14.2-4~pve6+1) pve; urgency=medium
 
   [ Proxmox Support Team ]
diff --git a/patches/CVE-2020-14355_1.patch b/patches/CVE-2020-14355_1.patch
new file mode 100644
index 0000000..c76847c
--- /dev/null
+++ b/patches/CVE-2020-14355_1.patch
@@ -0,0 +1,28 @@
+commit 762e0abae36033ccde658fd52d3235887b60862d
+Author: Frediano Ziglio <freddy77@gmail.com>
+Date:   Wed Apr 29 15:09:13 2020 +0100
+
+    quic: Check we have some data to start decoding quic image
+    
+    All paths already pass some data to quic_decode_begin but for the
+    test check it, it's not that expensive test.
+    Checking for not 0 is enough, all other words will potentially be
+    read calling more_io_words but we need one to avoid a potential
+    initial buffer overflow or deferencing an invalid pointer.
+    
+    Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
+    Acked-by: Uri Lublin <uril@redhat.com>
+
+diff --git a/subprojects/spice-common/quic.c b/subprojects/spice-common/quic.c
+index e2dee0f..bc753ca 100644
+--- a/subprojects/spice-common/common/quic.c
++++ b/subprojects/spice-common/common/quic.c
+@@ -1136,7 +1136,7 @@ int quic_decode_begin(QuicContext *quic, uint32_t *io_ptr, unsigned int num_io_w
+     int channels;
+     int bpc;
+ 
+-    if (!encoder_reset(encoder, io_ptr, io_ptr_end)) {
++    if (!num_io_words || !encoder_reset(encoder, io_ptr, io_ptr_end)) {
+         return QUIC_ERROR;
+     }
+ 
diff --git a/patches/CVE-2020-14355_2.patch b/patches/CVE-2020-14355_2.patch
new file mode 100644
index 0000000..6926286
--- /dev/null
+++ b/patches/CVE-2020-14355_2.patch
@@ -0,0 +1,43 @@
+commit 404d74782c8b5e57d146c5bf3118bb41bf3378e4
+Author: Frediano Ziglio <freddy77@gmail.com>
+Date:   Wed Apr 29 15:10:24 2020 +0100
+
+    quic: Check image size in quic_decode_begin
+    
+    Avoid some overflow in code due to images too big or
+    negative numbers.
+    
+    Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
+    Acked-by: Uri Lublin <uril@redhat.com>
+
+diff --git a/subprojects/spice-common/common/quic.c b/subprojects/spice-common/common/quic.c
+index bc753ca..6815316 100644
+--- a/subprojects/spice-common/common/quic.c
++++ b/subprojects/spice-common/common/quic.c
+@@ -56,6 +56,9 @@ typedef uint8_t BYTE;
+ #define MINwminext 1
+ #define MAXwminext 100000000
+ 
++/* Maximum image size in pixels, mainly to avoid possible integer overflows */
++#define SPICE_MAX_IMAGE_SIZE (512 * 1024 * 1024 - 1)
++
+ typedef struct QuicFamily {
+     unsigned int nGRcodewords[MAXNUMCODES];      /* indexed by code number, contains number of
+                                                     unmodified GR codewords in the code */
+@@ -1165,6 +1168,16 @@ int quic_decode_begin(QuicContext *quic, uint32_t *io_ptr, unsigned int num_io_w
+     height = encoder->io_word;
+     decode_eat32bits(encoder);
+ 
++    if (width <= 0 || height <= 0) {
++        encoder->usr->warn(encoder->usr, "invalid size\n");
++        return QUIC_ERROR;
++    }
++
++    /* avoid too big images */
++    if ((uint64_t) width * height > SPICE_MAX_IMAGE_SIZE) {
++        encoder->usr->error(encoder->usr, "image too large\n");
++    }
++
+     quic_image_params(encoder, type, &channels, &bpc);
+ 
+     if (!encoder_reset_channels(encoder, channels, width, bpc)) {
diff --git a/patches/CVE-2020-14355_3.patch b/patches/CVE-2020-14355_3.patch
new file mode 100644
index 0000000..77ea4ba
--- /dev/null
+++ b/patches/CVE-2020-14355_3.patch
@@ -0,0 +1,30 @@
+commit ef1b6ff7b82e15d759e5415b8e35b92bb1a4c206
+Author: Frediano Ziglio <freddy77@gmail.com>
+Date:   Wed Apr 29 15:11:38 2020 +0100
+
+    quic: Check RLE lengths
+    
+    Avoid buffer overflows decoding images. On compression we compute
+    lengths till end of line so it won't cause regressions.
+    Proved by fuzzing the code.
+    
+    Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
+    Acked-by: Uri Lublin <uril@redhat.com>
+
+diff --git a/subprojects/spice-common/common/quic_tmpl.c b/subprojects/spice-common/common/quic_tmpl.c
+index ecd6f3f..ebae992 100644
+--- a/subprojects/spice-common/common/quic_tmpl.c
++++ b/subprojects/spice-common/common/quic_tmpl.c
+@@ -563,7 +563,11 @@ static void FNAME_DECL(uncompress_row_seg)(const PIXEL * const prev_row,
+ do_run:
+         state->waitcnt = stopidx - i;
+         run_index = i;
+-        run_end = i + decode_state_run(encoder, state);
++        run_end = decode_state_run(encoder, state);
++        if (run_end < 0 || run_end > (end - i)) {
++            encoder->usr->error(encoder->usr, "wrong RLE\n");
++        }
++        run_end += i;
+ 
+         for (; i < run_end; i++) {
+             UNCOMPRESS_PIX_START(&cur_row[i]);
diff --git a/patches/CVE-2020-14355_4.patch b/patches/CVE-2020-14355_4.patch
new file mode 100644
index 0000000..64b039d
--- /dev/null
+++ b/patches/CVE-2020-14355_4.patch
@@ -0,0 +1,29 @@
+commit b24fe6b66b86e601c725d30f00c37e684b6395b6
+Author: Frediano Ziglio <freddy77@gmail.com>
+Date:   Thu Apr 30 10:19:09 2020 +0100
+
+    quic: Avoid possible buffer overflow in find_bucket
+    
+    Proved by fuzzing the code.
+    
+    Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
+    Acked-by: Uri Lublin <uril@redhat.com>
+
+diff --git a/subprojects/spice-common/common/quic_family_tmpl.c b/subprojects/spice-common/common/quic_family_tmpl.c
+index 8a5f7d2..6cc051b 100644
+--- a/subprojects/spice-common/common/quic_family_tmpl.c
++++ b/subprojects/spice-common/common/quic_family_tmpl.c
+@@ -103,7 +103,12 @@ static s_bucket *FNAME(find_bucket)(Channel *channel, const unsigned int val)
+ {
+     spice_extra_assert(val < (0x1U << BPC));
+ 
+-    return channel->_buckets_ptrs[val];
++    /* The and (&) here is to avoid buffer overflows in case of garbage or malicious
++     * attempts. Is much faster then using comparisons and save us from such situations.
++     * Note that on normal build the check above won't be compiled as this code path
++     * is pretty hot and would cause speed regressions.
++     */
++    return channel->_buckets_ptrs[val & ((1U << BPC) - 1)];
+ }
+ 
+ #undef FNAME
diff --git a/patches/red-qxl-make-sure-we-have-at-least-one-monitor.patch b/patches/red-qxl-make-sure-we-have-at-least-one-monitor.patch
deleted file mode 100644
index 407e986..0000000
--- a/patches/red-qxl-make-sure-we-have-at-least-one-monitor.patch
+++ /dev/null
@@ -1,33 +0,0 @@
-From 4f2d90a7849fafebc74dec608f9b4ffa9400d1a6 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Thu, 19 Sep 2019 11:17:08 +0100
-Subject: red-qxl: Make sure we have at least one monitor
-
-It does not make sense to have a graphic card without a monitor.
-In spice_qxl_set_max_monitors we prevent to set 0 monitors, do
-the same in spice_qxl_set_device_info.
-
-This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1691721.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
-Acked-by: Victor Toso <victortoso@redhat.com>
----
- server/red-qxl.c | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/server/red-qxl.c b/server/red-qxl.c
-index 0334827..dbfcd44 100644
---- a/server/red-qxl.c
-+++ b/server/red-qxl.c
-@@ -804,7 +804,7 @@ void spice_qxl_set_device_info(QXLInstance *instance,
-     }
- 
-     instance->st->monitors_count = device_display_id_count;
--    instance->st->max_monitors = device_display_id_count;
-+    instance->st->max_monitors = MAX(1u, device_display_id_count);
- 
-     reds_send_device_display_info(red_qxl_get_server(instance->st));
- }
--- 
-cgit v1.1
-- 
2.20.1