File 0001-CVE-2020-25650-Avoids-unchecked-file-transfer-IDs-allocation-and-usage.patch of Package spice-vdagent.20484

Subject: Avoids unchecked file transfer IDs allocation and usage
From: Frediano Ziglio freddy77@gmail.com Sat Sep 19 15:13:42 2020 +0100
Date: Thu Oct 29 14:59:18 2020 +0000:
Git: 1a8b93ca6ac0b690339ab7f0afc6fc45d198d332

Avoid agents allocating file transfers.
The "active_xfers" entries are now inserted when client start sending
files.
Also different agents cannot mess with other agent transfers as a
transfer is bound to a single agent.

This issue was reported by SUSE security team.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>

Index: spice-vdagent-0.17.0/src/vdagentd.c
===================================================================
--- spice-vdagent-0.17.0.orig/src/vdagentd.c
+++ spice-vdagent-0.17.0/src/vdagentd.c
@@ -341,9 +341,11 @@ static void do_client_file_xfer(struct v
                s->id, VD_AGENT_FILE_XFER_STATUS_ERROR);
             return;
         }
-        udscs_write(active_session_conn, VDAGENTD_FILE_XFER_START, 0, 0,
-                    data, message_header->size);
-        return;
+        msg_type = VDAGENTD_FILE_XFER_START;
+        id = s->id;
+        // associate the id with the active connection
+        g_hash_table_insert(active_xfers, GUINT_TO_POINTER(id), active_session_conn);
+        break;
     }
     case VD_AGENT_FILE_XFER_STATUS: {
         VDAgentFileXferStatusMessage *s = (VDAgentFileXferStatusMessage *)data;
@@ -368,6 +370,12 @@ static void do_client_file_xfer(struct v
         return;
     }
     udscs_write(conn, msg_type, 0, 0, data, message_header->size);
+
+    // client told that transfer is ended, agents too stop the transfer
+    // and release resources
+    if (message_header->type == VD_AGENT_FILE_XFER_STATUS) {
+        g_hash_table_remove(active_xfers, GUINT_TO_POINTER(id));
+    }
 }
 
 static gsize vdagent_message_min_size[] =
@@ -908,15 +916,23 @@ static void agent_read_complete(struct u
     case VDAGENTD_FILE_XFER_STATUS:{
         VDAgentFileXferStatusMessage status;
         status.id = GUINT32_TO_LE(header->arg1);
+	gpointer task_id = GUINT_TO_POINTER(status.id);
+        struct udscs_connection *task_conn = g_hash_table_lookup(active_xfers, task_id);
+        if (task_conn == NULL || task_conn != *connp) {
+            // Protect against misbehaving agent.
+            // Ignore the message, but do not disconnect the agent, to protect against
+            // a misbehaving client that tries to disconnect a good agent
+            // e.g. by sending a new task and immediately cancelling it.
+            return;
+        }
+
         status.result = GUINT32_TO_LE(header->arg2);
         vdagent_virtio_port_write(virtio_port, VDP_CLIENT_PORT,
                                   VD_AGENT_FILE_XFER_STATUS, 0,
                                   (uint8_t *)&status, sizeof(status));
-        if (status.result == VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA)
-            g_hash_table_insert(active_xfers, GUINT_TO_POINTER(status.id),
-                                *connp);
-        else
+        if (status.result != VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA) {
             g_hash_table_remove(active_xfers, GUINT_TO_POINTER(status.id));
+        }
         break;
     }
 
openSUSE Build Service is sponsored by