From 956608c1344f185e39294004b64906a7e1b5c14c Mon Sep 17 00:00:00 2001 From: Frediano Ziglio 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 Acked-by: Uri Lublin --- 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