[Spice-commits] 2 commits - src/spice-channel.c

[GitLab Mirror]

 src/spice-channel.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

New commits:
commit 05f19eb3c171cc5e491d079bcae25c3db38c40ef
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Nov 20 12:14:36 2018 +0000

    spice-channel: Avoid some buffer reading overflows
    
    Check link message contains valid offset and array sizes.
    The overflows do not produce data leaking as data are copied into
    other client arrays and used only for checking limited bit arrays.
    This remove possible client DoS.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/src/spice-channel.c b/src/spice-channel.c
index 7e5b2e7..cc089eb 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1906,7 +1906,7 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
     int rc;
     uint32_t num_caps;
     uint32_t num_channel_caps, num_common_caps;
-    uint8_t *caps_src;
+    const uint8_t *caps_src, *caps_end;
     SpiceChannelEvent event = SPICE_CHANNEL_ERROR_LINK;
 
     g_return_val_if_fail(channel != NULL, FALSE);
@@ -1947,14 +1947,25 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
     num_caps = num_channel_caps + num_common_caps;
     CHANNEL_DEBUG(channel, "%s: %u caps", __FUNCTION__, num_caps);
 
+    if (c->peer_msg->caps_offset > c->peer_hdr.size) {
+        goto error;
+    }
+    caps_end = (uint8_t*)c->peer_msg + c->peer_hdr.size;
+
     /* see original spice/client code: */
     /* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * sizeof(uint32_t) > c->peer_msg + c->peer_hdr.size); */
 
     caps_src = (uint8_t *)c->peer_msg + c->peer_msg->caps_offset;
+    if ((caps_end - caps_src) / sizeof(uint32_t) < num_common_caps) {
+        goto error;
+    }
     CHANNEL_DEBUG(channel, "got remote common caps:");
     store_caps(caps_src, num_common_caps, c->remote_common_caps);
 
     caps_src += num_common_caps * sizeof(uint32_t);
+    if ((caps_end - caps_src) / sizeof(uint32_t) < num_channel_caps) {
+        goto error;
+    }
     CHANNEL_DEBUG(channel, "got remote channel caps:");
     store_caps(caps_src, num_channel_caps, c->remote_caps);
 
commit fc89acd6f88d5e34ad18350aee00530293328601
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Nov 20 14:17:40 2018 +0000

    spice-channel: Check minumum size of peer_msg
    
    Other parts of the code assume peer_msg contains at least a fixed
    structure so make sure server is sending enough data.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/src/spice-channel.c b/src/spice-channel.c
index c61bcba..7e5b2e7 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1400,6 +1400,11 @@ static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel)
     c->peer_hdr.minor_version = GUINT32_FROM_LE(c->peer_hdr.minor_version);
     c->peer_hdr.size = GUINT32_FROM_LE(c->peer_hdr.size);
 
+    if (c->peer_hdr.size < sizeof(*c->peer_msg)) {
+        g_warning("invalid peer header size: %u", c->peer_hdr.size);
+        goto error;
+    }
+
     c->peer_msg = g_malloc0(c->peer_hdr.size);
     if (c->peer_msg == NULL) {
         g_warning("invalid peer header size: %u", c->peer_hdr.size);