Bug#898503: spice-gtk: CVE-2017-12194: Integer overflows causing buffer overflows in spice-client

[Laurent Bigonville]

Package: src:spice-gtk
Followup-For: Bug #898503

Hi,

This seems to be fixed in 0.35 release, could you please update? (It
requires a newer version of spice-protocol first)

Kind regards,

Laurent Bigonville

-- System Information:
Debian Release: buster/sid
  APT prefers unstable-debug
  APT policy: (500, 'unstable-debug'), (500, 'unstable'), (1, 
'experimental-debug'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.18.0-rc4-amd64 (SMP w/8 CPU cores)
Locale: LANG=fr_BE.UTF-8, LC_CTYPE=fr_BE.UTF-8 (charmap=UTF-8), 
LANGUAGE=fr_BE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: SELinux: enabled - Mode: Permissive - Policy name: refpolicy

Bug#898503: spice-gtk: CVE-2017-12194: Integer overflows causing buffer overflows in spice-client

[Salvatore Bonaccorso]

Control: tags -1 + patch

Attaching as well the two proposed patches (and which make the
testcase pass).

Regards,
Salvatore
>From 78b54cbaa064f0ac94af114edb54fca3b365430d Mon Sep 17 00:00:00 2001
From: Frediano Ziglio 
Date: Fri, 19 Jun 2015 14:42:54 +0100
Subject: [PATCH spice-common 1/3] Write a small test to test possible crash

This small test prove a that current generated demarshaller code
is not safe to integer overflows leading to buffer overflows.
Actually from a quick look at the protocol it seems that client
can't cause these overflows but server can quite easily at
demonstrated by this test.

Signed-off-by: Frediano Ziglio 
---
 tests/Makefile.am | 14 +
 tests/test-overflow.c | 80 +++
 2 files changed, 94 insertions(+)
 create mode 100644 tests/test-overflow.c

Index: spice-gtk-0.34/spice-common/tests/Makefile.am
===
--- spice-gtk-0.34.orig/spice-common/tests/Makefile.am
+++ spice-gtk-0.34/spice-common/tests/Makefile.am
@@ -63,4 +63,18 @@ EXTRA_DIST =\
 	test-marshallers.proto		\
 	$(NULL)
 
+TESTS += test_overflow
+test_overflow_SOURCES = test-overflow.c
+test_overflow_CFLAGS = \
+	-I$(top_srcdir) \
+	$(GLIB2_CFLAGS) \
+	$(SPICE_COMMON_CFLAGS) \
+	$(PROTOCOL_CFLAGS) \
+	$(NULL)
+test_overflow_LDADD = \
+	$(top_builddir)/common/libspice-common.la \
+	$(top_builddir)/common/libspice-common-server.la \
+	$(top_builddir)/common/libspice-common-client.la \
+	$(NULL)
+
 -include $(top_srcdir)/git.mk
Index: spice-gtk-0.34/spice-common/tests/test-overflow.c
===
--- /dev/null
+++ spice-gtk-0.34/spice-common/tests/test-overflow.c
@@ -0,0 +1,80 @@
+/*
+   Copyright (C) 2015 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see .
+*/
+#ifdef HAVE_CONFIG_H
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define NUM_CHANNELS 3u
+
+int main(void)
+{
+	SpiceMarshaller *m;
+	SpiceMsgChannels *msg;
+	uint8_t *data, *out;
+	size_t len;
+	int to_free = 0;
+	spice_parse_channel_func_t func;
+	unsigned int max_message_type, n;
+	message_destructor_t free_output;
+
+	m = spice_marshaller_new();
+	assert(m);
+
+	msg = (SpiceMsgChannels *) malloc(sizeof(SpiceMsgChannels) +
+	  NUM_CHANNELS * sizeof(SpiceChannelId));
+	assert(msg);
+
+	// build a message and marshal it
+	msg->num_of_channels = NUM_CHANNELS;
+	for (n = 0; n < NUM_CHANNELS; ++n)
+		msg->channels[n] = (SpiceChannelId) { n + 1, n * 7 };
+	spice_marshall_msg_main_channels_list(m, msg);
+
+	// get linear data
+	data = spice_marshaller_linearize(m, 0, , _free);
+	assert(data);
+
+	printf("output len %lu\n", (unsigned long) len);
+
+	// hack, try to core
+	*((uint32_t *) data) = 0x8002u;
+
+	// extract the message
+	func = spice_get_server_channel_parser(1, _message_type);
+	assert(func);
+	out = func(data, data+len, SPICE_MSG_MAIN_CHANNELS_LIST, 0, , _output);
+	assert(out == NULL);
+
+	// cleanup
+	if (to_free)
+		free(data);
+	if (out)
+		free_output(out);
+	free(msg);
+
+	return 0;
+}
+
Index: spice-gtk-0.34/spice-common/tests/Makefile.in
===
--- spice-gtk-0.34.orig/spice-common/tests/Makefile.in
+++ spice-gtk-0.34/spice-common/tests/Makefile.in
@@ -1,7 +1,7 @@
-# Makefile.in generated by automake 1.15 from Makefile.am.
+# Makefile.in generated by automake 1.15.1 from Makefile.am.
 # @configure_input@
 
-# Copyright (C) 1994-2014 Free Software Foundation, Inc.
+# Copyright (C) 1994-2017 Free Software Foundation, Inc.
 
 # This Makefile.in is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
@@ -88,7 +88,8 @@ PRE_UNINSTALL = :
 POST_UNINSTALL = :
 build_triplet = @build@
 host_triplet = @host@
-TESTS = test_logging$(EXEEXT) test_marshallers$(EXEEXT)
+TESTS = test_logging$(EXEEXT) test_marshallers$(EXEEXT) \
+	test_overflow$(EXEEXT)
 noinst_PROGRAMS = $(am__EXEEXT_1)
 subdir = tests
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
@@ -104,7 +105,8 @@ mkinstalldirs = $(install_sh) -d
 CONFIG_HEADER = $(top_builddir)/config.h
 CONFIG_CLEAN_FILES =
 CONFIG_CLEAN_VPATH_FILES =
-am__EXEEXT_1 = test_logging$(EXEEXT) test_marshallers$(EXEEXT)

Bug#898503: spice-gtk: CVE-2017-12194: Integer overflows causing buffer overflows in spice-client

[Salvatore Bonaccorso]

Source: spice-gtk
Version: 0.25-1
Severity: important
Tags: security upstream

Hi,

The following vulnerability was published for spice-gtk.

CVE-2017-12194[0]:
| A flaw was found in the way spice-client processed certain messages
| sent from the server. An attacker, having control of malicious
| spice-server, could use this flaw to crash the client or execute
| arbitrary code with permissions of the user running the client.
| spice-gtk versions through 0.34 are believed to be vulnerable.

See [2] for a test-program to demostrate the issue (attached here as
well) and two proposed patches to be applied.

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-2017-12194
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-12194
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1501200
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1240165

Regards,
Salvatore
>From 78b54cbaa064f0ac94af114edb54fca3b365430d Mon Sep 17 00:00:00 2001
From: Frediano Ziglio 
Date: Fri, 19 Jun 2015 14:42:54 +0100
Subject: [PATCH spice-common 1/3] Write a small test to test possible crash

This small test prove a that current generated demarshaller code
is not safe to integer overflows leading to buffer overflows.
Actually from a quick look at the protocol it seems that client
can't cause these overflows but server can quite easily at
demonstrated by this test.

Signed-off-by: Frediano Ziglio 
---
 tests/Makefile.am | 14 +
 tests/test-overflow.c | 80 +++
 2 files changed, 94 insertions(+)
 create mode 100644 tests/test-overflow.c

Index: spice-gtk-0.34/spice-common/tests/Makefile.am
===
--- spice-gtk-0.34.orig/spice-common/tests/Makefile.am
+++ spice-gtk-0.34/spice-common/tests/Makefile.am
@@ -63,4 +63,18 @@ EXTRA_DIST = \
test-marshallers.proto  \
$(NULL)
 
+TESTS += test_overflow
+test_overflow_SOURCES = test-overflow.c
+test_overflow_CFLAGS = \
+   -I$(top_srcdir) \
+   $(GLIB2_CFLAGS) \
+   $(SPICE_COMMON_CFLAGS) \
+   $(PROTOCOL_CFLAGS) \
+   $(NULL)
+test_overflow_LDADD = \
+   $(top_builddir)/common/libspice-common.la \
+   $(top_builddir)/common/libspice-common-server.la \
+   $(top_builddir)/common/libspice-common-client.la \
+   $(NULL)
+
 -include $(top_srcdir)/git.mk
Index: spice-gtk-0.34/spice-common/tests/test-overflow.c
===
--- /dev/null
+++ spice-gtk-0.34/spice-common/tests/test-overflow.c
@@ -0,0 +1,80 @@
+/*
+   Copyright (C) 2015 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see .
+*/
+#ifdef HAVE_CONFIG_H
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define NUM_CHANNELS 3u
+
+int main(void)
+{
+   SpiceMarshaller *m;
+   SpiceMsgChannels *msg;
+   uint8_t *data, *out;
+   size_t len;
+   int to_free = 0;
+   spice_parse_channel_func_t func;
+   unsigned int max_message_type, n;
+   message_destructor_t free_output;
+
+   m = spice_marshaller_new();
+   assert(m);
+
+   msg = (SpiceMsgChannels *) malloc(sizeof(SpiceMsgChannels) +
+ NUM_CHANNELS * sizeof(SpiceChannelId));
+   assert(msg);
+
+   // build a message and marshal it
+   msg->num_of_channels = NUM_CHANNELS;
+   for (n = 0; n < NUM_CHANNELS; ++n)
+   msg->channels[n] = (SpiceChannelId) { n + 1, n * 7 };
+   spice_marshall_msg_main_channels_list(m, msg);
+
+   // get linear data
+   data = spice_marshaller_linearize(m, 0, , _free);
+   assert(data);
+
+   printf("output len %lu\n", (unsigned long) len);
+
+   // hack, try to core
+   *((uint32_t *) data) = 0x8002u;
+
+   // extract the message
+   func = spice_get_server_channel_parser(1, _message_type);
+   assert(func);
+   out = func(data, data+len, SPICE_MSG_MAIN_CHANNELS_LIST, 0, , 
_output);
+   assert(out == NULL);
+
+   // cleanup
+   if (to_free)
+   free(data);
+   if (out)
+