File Avoids-unchecked-file-transfer-IDs-allocation-a... of Package spice-vdagent.17026

Overview Repositories Revisions Requests Users Attributes Meta

File Avoids-unchecked-file-transfer-IDs-allocation-and-us.patch of Package spice-vdagent.17026

From eff122eca488986543789b3944f87f9fda620a34 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <freddy77@gmail.com>
Date: Sat, 19 Sep 2020 15:13:42 +0100
Subject: [PATCH 13/20] Avoids unchecked file transfer IDs allocation and usage

Git-commit: 6923e654daa6842723675f36c8f75ebd461a7ee5
References: bsc#1173749

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>
Signed-off-by: Bruce Rogers <brogers@suse.com>
---
 src/vdagentd/vdagentd.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index 963c030..52b7855 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -359,9 +359,11 @@ static void do_client_file_xfer(struct vdagent_virtio_port *vport,
                s->id, VD_AGENT_FILE_XFER_STATUS_SESSION_LOCKED, NULL, 0);
             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;
@@ -386,6 +388,12 @@ static void do_client_file_xfer(struct vdagent_virtio_port *vport,
         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 void forward_data_to_session_agent(uint32_t type, uint8_t *data, size_t size)
@@ -976,6 +984,15 @@ static void do_agent_file_xfer_status(struct udscs_connection     *conn,
     const gchar *log_msg = NULL;
     guint data_size = 0;
 
+    UdscsConnection *task_conn = g_hash_table_lookup(active_xfers, task_id);
+    if (task_conn == NULL || task_conn != conn) {
+        // 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;
+    }
+
     /* header->arg1 = file xfer task id, header->arg2 = file xfer status */
     switch (header->arg2) {
         case VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE:
@@ -990,10 +1007,9 @@ static void do_agent_file_xfer_status(struct udscs_connection     *conn,
     send_file_xfer_status(virtio_port, log_msg, header->arg1, header->arg2,
                           data, data_size);
 
-    if (header->arg2 == VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA)
-        g_hash_table_insert(active_xfers, task_id, conn);
-    else
+    if (header->arg2 != VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA) {
         g_hash_table_remove(active_xfers, task_id);
+    }
 }
 
 static void agent_read_complete(struct udscs_connection *conn,
-- 
2.29.0

Places

File Avoids-unchecked-file-transfer-IDs-allocation-and-us.patch of Package spice-vdagent.17026

Places