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

[GitLab Mirror]

 src/channel-main.c |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

New commits:
commit 9b98e01c8f5d0dc8faaf3af7b8fc95768e1ff0ad
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Wed Sep 16 15:50:33 2020 +0100

    channel-main: Handle not terminated host_data and cert_subject_data fields
    
    host_data and cert_subject_data fields from SPICE messages could be
    not NUL terminated so using g_strdup can lead to some read overflow.
    
    This bug was discovered by Uri Lublin.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Uri Lublin <uril at redhat.com>

diff --git a/src/channel-main.c b/src/channel-main.c
index 2881d59..5fefded 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -2460,10 +2460,11 @@ static void main_migrate_connect(SpiceChannel *channel,
     mig->src_channel = channel;
     mig->info = *dst_info;
     if (dst_info->host_data) {
-        mig->info.host_data = (void *) g_strdup((char*) dst_info->host_data);
+        mig->info.host_data = (void *) g_strndup((char*) dst_info->host_data, dst_info->host_size);
     }
     if (dst_info->cert_subject_data) {
-        mig->info.cert_subject_data = (void *) g_strdup((char*) dst_info->cert_subject_data);
+        mig->info.cert_subject_data = (void *) g_strndup((char*) dst_info->cert_subject_data,
+                                                         dst_info->cert_subject_size);
     }
     mig->from = coroutine_self();
     mig->do_seamless = do_seamless;
commit 1f2a7a079a42ac9bccc12749c5eac4fcdbd48b2e
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Wed Sep 16 17:12:14 2020 +0100

    channel-main: Make more clear that host_data and cert_subject_data are C strings
    
    After commit ab42be2b00d12d0bc98c6ddea08a7f969e83b2ac ("channel-main:
    Copy SpiceMigrationDstInfo into spice_migrate") host_data and
    cert_subject_data fields in spice_migrate structure are proper
    terminated C strings so:
    - check pointer instead of related field;
    - you don't need to terminate again.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Uri Lublin <uril at redhat.com>

diff --git a/src/channel-main.c b/src/channel-main.c
index 5f81975..2881d59 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -2412,18 +2412,14 @@ static gboolean migrate_connect(spice_migrate *mig)
     sport = info->sport;
     host = (char*)info->host_data;
 
-    if (info->cert_subject_size == 0 ||
+    if (info->cert_subject_data == NULL ||
         strlen((const char*)info->cert_subject_data) == 0) {
         /* only verify hostname if no cert subject */
         g_object_set(mig->session, "verify", SPICE_SESSION_VERIFY_HOSTNAME, NULL);
     } else {
-        gchar *subject = g_alloca(info->cert_subject_size + 1);
-        strncpy(subject, (const char*)info->cert_subject_data, info->cert_subject_size);
-        subject[info->cert_subject_size] = '\0';
-
         // session data are already copied
         g_object_set(mig->session,
-                     "cert-subject", subject,
+                     "cert-subject", info->cert_subject_data,
                      "verify", SPICE_SESSION_VERIFY_SUBJECT,
                      NULL);
     }