From 956608c1344f185e39294004b64906a7e1b5c14c Mon Sep 17 00:00:00 2001

From: Frediano Ziglio <freddy77@gmail.com>

Date: Sat, 19 Sep 2020 15:13:42 +0100

Subject: [PATCH vd_agent_linux 10/17] Avoids unchecked file transfer IDs

 allocation and usage

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>

---

 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 eddfcf6..8961a99 100644

--- a/src/vdagentd/vdagentd.c

+++ b/src/vdagentd/vdagentd.c

@@ -381,9 +381,11 @@ static void do_client_file_xfer(VirtioPort *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;

@@ -408,6 +410,12 @@ static void do_client_file_xfer(VirtioPort *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)

@@ -1015,6 +1023,15 @@ static void do_agent_file_xfer_status(UdscsConnection             *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:

@@ -1029,10 +1046,9 @@ static void do_agent_file_xfer_status(UdscsConnection             *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(UdscsConnection *conn,

-- 

2.26.2