From 587c04f79ee8b6d105063391c8b07c01715a5cc6 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Wed, 16 Sep 2020 08:35:18 +0100 Subject: [PATCH] Improve big endian support for agent messages Big endian machines on server are not officially supported but won't hurt. Messages from client are always encoded as little endian (as all SPICE protocol). Convert fields from little endian to host endian on some places where numbers are used and not just binary copied. Signed-off-by: Frediano Ziglio Acked-by: Uri Lublin --- server/agent-msg-filter.c | 3 +++ server/reds.cpp | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c index 11612c6b..e18e8fcf 100644 --- a/server/agent-msg-filter.c +++ b/server/agent-msg-filter.c @@ -70,6 +70,9 @@ data_to_read: return AGENT_MSG_FILTER_PROTO_ERROR; } memcpy(&msg_header, data, sizeof(msg_header)); + msg_header.protocol = GUINT32_FROM_LE(msg_header.protocol); + msg_header.type = GUINT32_FROM_LE(msg_header.type); + msg_header.size = GUINT32_FROM_LE(msg_header.size); len -= sizeof(msg_header); if (msg_header.protocol != VD_AGENT_PROTOCOL) { diff --git a/server/reds.cpp b/server/reds.cpp index fd3632f0..cea9a4e9 100644 --- a/server/reds.cpp +++ b/server/reds.cpp @@ -1119,6 +1119,7 @@ static void reds_on_main_agent_monitors_config(RedsState *reds, size_t monitor_size = sizeof(VDAgentMonConfig); SpiceBuffer *cmc = &reds->client_monitors_config; uint32_t max_monitors; + uint32_t msg_size; // limit size of message sent by the client as this can cause a DoS through // memory exhaustion, or potentially some integer overflows @@ -1131,16 +1132,24 @@ static void reds_on_main_agent_monitors_config(RedsState *reds, return; } msg_header = (VDAgentMessage *)cmc->buffer; - if (msg_header->size > MAX_MONITOR_CONFIG_SIZE) { + msg_size = GUINT32_FROM_LE(msg_header->size); + if (msg_size > MAX_MONITOR_CONFIG_SIZE) { goto overflow; } - if (msg_header->size > cmc->offset - sizeof(VDAgentMessage)) { + if (msg_size > cmc->offset - sizeof(VDAgentMessage)) { spice_debug("not enough data yet. %" G_GSSIZE_FORMAT, cmc->offset); return; } - if (msg_header->size < sizeof(VDAgentMonitorsConfig)) { + if (msg_size < sizeof(VDAgentMonitorsConfig)) { goto overflow; } + + // convert VDAgentMessage endianness + msg_header->protocol = GUINT32_FROM_LE(msg_header->protocol); + msg_header->type = GUINT32_FROM_LE(msg_header->type); + msg_header->opaque = GUINT64_FROM_LE(msg_header->opaque); + msg_header->size = GUINT32_FROM_LE(msg_header->size); + monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + sizeof(*msg_header)); /* filter out not known flags */ monitors_config->flags &= VD_AGENT_CONFIG_MONITORS_FLAG_USE_POS | @@ -1149,7 +1158,7 @@ static void reds_on_main_agent_monitors_config(RedsState *reds, monitor_size += sizeof(VDAgentMonitorMM); } // limit the monitor number to avoid buffer overflows - max_monitors = (msg_header->size - sizeof(VDAgentMonitorsConfig)) / + max_monitors = (msg_size - sizeof(VDAgentMonitorsConfig)) / monitor_size; if (monitors_config->num_of_monitors > max_monitors) { goto overflow;