Commit 087e7049 authored by Peter Korsgaard's avatar Peter Korsgaard
Browse files

spice: add post-0.12.8 upstream security fixes

Fixes the following security issues:

CVE-2016-9577

    Frediano Ziglio of Red Hat discovered a buffer overflow
    vulnerability in the main_channel_alloc_msg_rcv_buf function. An
    authenticated attacker can take advantage of this flaw to cause a
    denial of service (spice server crash), or possibly, execute
    arbitrary code.

CVE-2016

-9578

    Frediano Ziglio of Red Hat discovered that spice does not properly
    validate incoming messages. An attacker able to connect to the
    spice server could send crafted messages which would cause the
    process to crash.

Signed-off-by: default avatarPeter Korsgaard <peter@korsgaard.com>
Reviewed-by: default avatar"Yann E. MORIN" <yann.morin.1998@free.fr>
Signed-off-by: default avatarPeter Korsgaard <peter@korsgaard.com>
parent 75057fe7
From 1c6517973095a67c8cb57f3550fc1298404ab556 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@redhat.com>
Date: Tue, 13 Dec 2016 14:39:48 +0000
Subject: [PATCH] Prevent possible DoS attempts during protocol handshake
The limit for link message is specified using a 32 bit unsigned integer.
This could cause possible DoS due to excessive memory allocations and
some possible crashes.
For instance a value >= 2^31 causes a spice_assert to be triggered in
async_read_handler (reds-stream.c) due to an integer overflow at this
line:
int n = async->end - async->now;
This could be easily triggered with a program like
#!/usr/bin/env python
import socket
import time
from struct import pack
server = '127.0.0.1'
port = 5900
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.connect((server, port))
data = pack('<4sIII', 'REDQ', 2, 2, 0xaaaaaaaa)
s.send(data)
time.sleep(1)
without requiring any authentication (the same can be done
with TLS).
[Peter: fixes CVE-2016-9578]
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
server/reds.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/server/reds.c b/server/reds.c
index f40b65c1..86a33d53 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2202,7 +2202,8 @@ static void reds_handle_read_header_done(void *opaque)
reds->peer_minor_version = header->minor_version;
- if (header->size < sizeof(SpiceLinkMess)) {
+ /* the check for 4096 is to avoid clients to cause arbitrary big memory allocations */
+ if (header->size < sizeof(SpiceLinkMess) || header->size > 4096) {
reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
spice_warning("bad size %u", header->size);
reds_link_free(link);
--
2.11.0
From f66dc643635518e53dfbe5262f814a64eec54e4a Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@redhat.com>
Date: Tue, 13 Dec 2016 14:40:10 +0000
Subject: [PATCH] Prevent integer overflows in capability checks
The limits for capabilities are specified using 32 bit unsigned integers.
This could cause possible integer overflows causing buffer overflows.
For instance the sum of num_common_caps and num_caps can be 0 avoiding
additional checks.
As the link message is now capped to 4096 and the capabilities are
contained in the link message limit the capabilities to 1024
(capabilities are expressed in number of uint32_t items).
[Peter: fixes CVE-2016-9578]
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
server/reds.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/server/reds.c b/server/reds.c
index 86a33d53..91504544 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2110,6 +2110,14 @@ static void reds_handle_read_link_done(void *opaque)
link_mess->num_channel_caps = GUINT32_FROM_LE(link_mess->num_channel_caps);
link_mess->num_common_caps = GUINT32_FROM_LE(link_mess->num_common_caps);
+ /* Prevent DoS. Currently we defined only 13 capabilities,
+ * I expect 1024 to be valid for quite a lot time */
+ if (link_mess->num_channel_caps > 1024 || link_mess->num_common_caps > 1024) {
+ reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
+ reds_link_free(link);
+ return;
+ }
+
num_caps = link_mess->num_common_caps + link_mess->num_channel_caps;
caps = (uint32_t *)((uint8_t *)link_mess + link_mess->caps_offset);
--
2.11.0
From 5f96b596353d73bdf4bb3cd2de61e48a7fd5b4c3 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@redhat.com>
Date: Tue, 29 Nov 2016 16:46:56 +0000
Subject: [PATCH] main-channel: Prevent overflow reading messages from client
Caller is supposed the function return a buffer able to store
size bytes.
[Peter: fixes CVE-2016-9577]
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
server/main_channel.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/server/main_channel.c b/server/main_channel.c
index 0ecc9df8..1fc39155 100644
--- a/server/main_channel.c
+++ b/server/main_channel.c
@@ -1026,6 +1026,9 @@ static uint8_t *main_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
if (type == SPICE_MSGC_MAIN_AGENT_DATA) {
return reds_get_agent_data_buffer(mcc, size);
+ } else if (size > sizeof(main_chan->recv_buf)) {
+ /* message too large, caller will log a message and close the connection */
+ return NULL;
} else {
return main_chan->recv_buf;
}
--
2.11.0
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment