Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [day] [month] [year] [list]
Date: Fri, 14 Jul 2017 09:36:21 +0200
From: Kristian Fiskerstrand <k_f@...too.org>
To: oss-security@...ts.openwall.com
Subject: CVE-2017-7506 spice: Possible buffer overflow via invalid monitor
 configurations

The following issue was brought to the distros list during the embargo
period. As per list policy this is the mandatory oss-security posting.

###

> CVE-2017-7506 spice: Possible buffer overflow via invalid monitor
configurations

CVSSv3:  9.1/CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:C/C:L/I:L/A:H

The vulnerability is exposed to authenticated clients.  Malicious SPICE
protocol messages can cause memory exhaustion, leak host memory to the
guest or cause OOB writes.  The writes seem difficult to control, but a
sufficiently crafty attacker could potentially use these to compromise
the host.

This was reported by SPICE maintainers Frediano Ziglio and Christophe
Fergeau, who also provided the attached patchset against the current
stable branch (0.12).

https://bugzilla.redhat.com/show_bug.cgi?id=1452606

-- 
Kristian Fiskerstrand
OpenPGP keyblock reachable at hkp://pool.sks-keyservers.net
fpr:94CB AFDD 3034 5109 5618 35AA 0B7F 8B60 E3ED FAE3

From 257f69d619fed407493156c8a7b952abc8a51314 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@...hat.com>
Date: Mon, 15 May 2017 15:57:28 +0100
Subject: [spice-server 1/3] reds: Disconnect when receiving overly big
 ClientMonitorsConfig

Total message size received from the client was unlimited. There is
a 2kiB size check on individual agent messages, but the MonitorsConfig
message can be split in multiple chunks, and the size of the
non-chunked MonitorsConfig message was never checked. This could easily
lead to memory exhaustion on the host.

Signed-off-by: Frediano Ziglio <fziglio@...hat.com>
---
 server/reds.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index f439a3668..7be85fdfc 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -993,19 +993,34 @@ static void reds_client_monitors_config_cleanup(void)
 static void reds_on_main_agent_monitors_config(
         MainChannelClient *mcc, void *message, size_t size)
 {
+    const unsigned int MAX_MONITORS = 256;
+    const unsigned int MAX_MONITOR_CONFIG_SIZE =
+       sizeof(VDAgentMonitorsConfig) + MAX_MONITORS * sizeof(VDAgentMonConfig);
+
     VDAgentMessage *msg_header;
     VDAgentMonitorsConfig *monitors_config;
     RedsClientMonitorsConfig *cmc = &reds->client_monitors_config;
 
+    // limit size of message sent by the client as this can cause a DoS through
+    // memory exhaustion, or potentially some integer overflows
+    if (sizeof(VDAgentMessage) + MAX_MONITOR_CONFIG_SIZE - cmc->buffer_size < size) {
+        goto overflow;
+    }
     cmc->buffer_size += size;
     cmc->buffer = realloc(cmc->buffer, cmc->buffer_size);
     spice_assert(cmc->buffer);
     cmc->mcc = mcc;
     memcpy(cmc->buffer + cmc->buffer_pos, message, size);
     cmc->buffer_pos += size;
+    if (sizeof(VDAgentMessage) > cmc->buffer_size) {
+        spice_debug("not enough data yet. %d", cmc->buffer_size);
+        return;
+    }
     msg_header = (VDAgentMessage *)cmc->buffer;
-    if (sizeof(VDAgentMessage) > cmc->buffer_size ||
-            msg_header->size > cmc->buffer_size - sizeof(VDAgentMessage)) {
+    if (msg_header->size > MAX_MONITOR_CONFIG_SIZE) {
+        goto overflow;
+    }
+    if (msg_header->size > cmc->buffer_size - sizeof(VDAgentMessage)) {
         spice_debug("not enough data yet. %d", cmc->buffer_size);
         return;
     }
@@ -1013,6 +1028,12 @@ static void reds_on_main_agent_monitors_config(
     spice_debug("%s: %d", __func__, monitors_config->num_of_monitors);
     red_dispatcher_client_monitors_config(monitors_config);
     reds_client_monitors_config_cleanup();
+    return;
+
+overflow:
+    spice_warning("received invalid MonitorsConfig request from client, disconnecting");
+    red_channel_client_disconnect(main_channel_client_get_base(mcc));
+    reds_client_monitors_config_cleanup();
 }
 
 void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t size)
-- 
2.13.0

From ff2b4ef70181087d5abd50bad76d026ec5088a93 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@...hat.com>
Date: Mon, 15 May 2017 15:57:28 +0100
Subject: [spice-server 2/3] reds: Avoid integer overflows handling monitor
 configuration

Avoid VDAgentMessage::size integer overflows.

Signed-off-by: Frediano Ziglio <fziglio@...hat.com>
---
 server/reds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/server/reds.c b/server/reds.c
index 7be85fdfc..e1c8c1086 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1024,6 +1024,9 @@ static void reds_on_main_agent_monitors_config(
         spice_debug("not enough data yet. %d", cmc->buffer_size);
         return;
     }
+    if (msg_header->size < sizeof(VDAgentMonitorsConfig)) {
+        goto overflow;
+    }
     monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + sizeof(*msg_header));
     spice_debug("%s: %d", __func__, monitors_config->num_of_monitors);
     red_dispatcher_client_monitors_config(monitors_config);
-- 
2.13.0

From 8cc3d7df2792751939cc832f4110c57e2addfca5 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@...hat.com>
Date: Mon, 15 May 2017 15:57:28 +0100
Subject: [spice-server 3/3] reds: Avoid buffer overflows handling monitor
 configuration

It was also possible for a malicious client to set
VDAgentMonitorsConfig::num_of_monitors to a number larger
than the actual size of VDAgentMOnitorsConfig::monitors.
This would lead to buffer overflows, which could allow the guest to
read part of the host memory. This might cause write overflows in the
host as well, but controlling the content of such buffers seems
complicated.

Signed-off-by: Frediano Ziglio <fziglio@...hat.com>
---
 server/reds.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/server/reds.c b/server/reds.c
index e1c8c1086..3a42c3755 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1000,6 +1000,7 @@ static void reds_on_main_agent_monitors_config(
     VDAgentMessage *msg_header;
     VDAgentMonitorsConfig *monitors_config;
     RedsClientMonitorsConfig *cmc = &reds->client_monitors_config;
+    uint32_t max_monitors;
 
     // limit size of message sent by the client as this can cause a DoS through
     // memory exhaustion, or potentially some integer overflows
@@ -1028,6 +1029,12 @@ static void reds_on_main_agent_monitors_config(
         goto overflow;
     }
     monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + sizeof(*msg_header));
+    // limit the monitor number to avoid buffer overflows
+    max_monitors = (msg_header->size - sizeof(VDAgentMonitorsConfig)) /
+                   sizeof(VDAgentMonConfig);
+    if (monitors_config->num_of_monitors > max_monitors) {
+        goto overflow;
+    }
     spice_debug("%s: %d", __func__, monitors_config->num_of_monitors);
     red_dispatcher_client_monitors_config(monitors_config);
     reds_client_monitors_config_cleanup();
-- 
2.13.0




[ CONTENT OF TYPE application/pgp-signature SKIPPED ]

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ