Processed: Re: Bug#797976: spice: CVE-2015-3247: memory corruption in worker_update_monitors_config()

[Debian Bug Tracking System]

Processing control commands:

> tags -1 + patch
Bug #797976 [src:spice] spice: CVE-2015-3247: memory corruption in 
worker_update_monitors_config()
Ignoring request to alter tags of bug #797976 to the same tags previously set

-- 
797976: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=797976
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

Bug#797976: spice: CVE-2015-3247: memory corruption in worker_update_monitors_config()

[Salvatore Bonaccorso]

Control: tags -1 + patch

Hi,

Attached is the debdiff prepared for a jessie-security upload.

Regards,
Salvatore
diff -Nru spice-0.12.5/debian/changelog spice-0.12.5/debian/changelog
--- spice-0.12.5/debian/changelog   2014-05-23 17:56:48.0 +0200
+++ spice-0.12.5/debian/changelog   2015-09-04 09:35:09.0 +0200
@@ -1,3 +1,12 @@
+spice (0.12.5-1+deb8u1) jessie-security; urgency=high
+
+  * Non-maintainer upload by the Security Team.
+  * Add CVE-2015-3247.patch patch.
+CVE-2015-3247: memory corruption in worker_update_monitors_config().
+(Closes: #797976)
+
+ -- Salvatore Bonaccorso   Fri, 04 Sep 2015 09:34:00 +0200
+
 spice (0.12.5-1) unstable; urgency=medium
 
   * new upstream release.  Can now build without celt!
diff -Nru spice-0.12.5/debian/patches/CVE-2015-3247.patch 
spice-0.12.5/debian/patches/CVE-2015-3247.patch
--- spice-0.12.5/debian/patches/CVE-2015-3247.patch 1970-01-01 
01:00:00.0 +0100
+++ spice-0.12.5/debian/patches/CVE-2015-3247.patch 2015-09-04 
09:35:09.0 +0200
@@ -0,0 +1,115 @@
+From 524eef10c6c6c2f3f30be28c56b8f96adc7901f0 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio 
+Date: Tue, 9 Jun 2015 08:50:46 +0100
+Subject: [PATCH] Avoid race conditions reading monitor configs from guest
+
+For security reasons do not assume guest do not change structures it
+pass to Qemu.
+Guest could change count field while Qemu is copying QXLMonitorsConfig
+structure leading to heap corruption.
+This patch avoid it reading count only once.
+
+Signed-off-by: Frediano Ziglio 
+---
+ server/red_worker.c | 46 --
+ 1 file changed, 32 insertions(+), 14 deletions(-)
+
+--- a/server/red_worker.c
 b/server/red_worker.c
+@@ -11051,7 +11051,8 @@ static inline void red_monitors_config_i
+ }
+ 
+ static void worker_update_monitors_config(RedWorker *worker,
+-  QXLMonitorsConfig 
*dev_monitors_config)
++  QXLMonitorsConfig 
*dev_monitors_config,
++  uint16_t count, uint16_t 
max_allowed)
+ {
+ int heads_size;
+ MonitorsConfig *monitors_config;
+@@ -11060,22 +11061,22 @@ static void worker_update_monitors_confi
+ monitors_config_decref(worker->monitors_config);
+ 
+ spice_debug("monitors config %d(%d)",
+-dev_monitors_config->count,
+-dev_monitors_config->max_allowed);
+-for (i = 0; i < dev_monitors_config->count; i++) {
++count,
++max_allowed);
++for (i = 0; i < count; i++) {
+ spice_debug("+%d+%d %dx%d",
+ dev_monitors_config->heads[i].x,
+ dev_monitors_config->heads[i].y,
+ dev_monitors_config->heads[i].width,
+ dev_monitors_config->heads[i].height);
+ }
+-heads_size = dev_monitors_config->count * sizeof(QXLHead);
++heads_size = count * sizeof(QXLHead);
+ worker->monitors_config = monitors_config =
+ spice_malloc(sizeof(*monitors_config) + heads_size);
+ monitors_config->refs = 1;
+ monitors_config->worker = worker;
+-monitors_config->count = dev_monitors_config->count;
+-monitors_config->max_allowed = dev_monitors_config->max_allowed;
++monitors_config->count = count;
++monitors_config->max_allowed = max_allowed;
+ memcpy(monitors_config->heads, dev_monitors_config->heads, heads_size);
+ }
+ 
+@@ -11459,33 +11460,50 @@ void handle_dev_display_migrate(void *op
+ red_migrate_display(worker, rcc);
+ }
+ 
++static inline uint32_t qxl_monitors_config_size(uint32_t heads)
++{
++return sizeof(QXLMonitorsConfig) + sizeof(QXLHead) * heads;
++}
++
+ static void handle_dev_monitors_config_async(void *opaque, void *payload)
+ {
+ RedWorkerMessageMonitorsConfigAsync *msg = payload;
+ RedWorker *worker = opaque;
+-int min_size = sizeof(QXLMonitorsConfig) + sizeof(QXLHead);
+ int error;
++uint16_t count, max_allowed;
+ QXLMonitorsConfig *dev_monitors_config =
+ (QXLMonitorsConfig*)get_virt(>mem_slots, msg->monitors_config,
+- min_size, msg->group_id, );
++ qxl_monitors_config_size(1),
++ msg->group_id, );
+ 
+ if (error) {
+ /* TODO: raise guest bug (requires added QXL interface) */
+ return;
+ }
+ worker->driver_cap_monitors_config = 1;
+-if (dev_monitors_config->count == 0) {
++count = dev_monitors_config->count;
++max_allowed = dev_monitors_config->max_allowed;
++if (count == 0) {
+ spice_warning("ignoring an empty monitors config message from 
driver");
+ return;
+ }
+-if (dev_monitors_config->count > dev_monitors_config->max_allowed) {
++if (count > max_allowed) {
+ spice_warning("ignoring malformed 

Bug#797976: spice: CVE-2015-3247: memory corruption in worker_update_monitors_config()

[Salvatore Bonaccorso]

Source: spice
Version: 0.12.5-1
Severity: grave
Tags: security patch upstream

Hi,

the following vulnerability was published for spice.

CVE-2015-3247[0]:
memory corruption in worker_update_monitors_config()

If you fix the vulnerability please also make sure to include the
CVE (Common Vulnerabilities & Exposures) id in your changelog entry.

For further information see:

[0] https://security-tracker.debian.org/tracker/CVE-2015-3247
[1] 
https://git.centos.org/blob/rpms!spice.git/11e32f6dd156a3c4847da29d989837437e973ccc/SOURCES!0038-Avoid-race-conditions-reading-monitor-configs-from-g.patch

Regards,
Salvatore