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;
}