[Spice-devel] [spice-server 1/3] reds: Disconnect when receiving overly big ClientMonitorsConfig

[Christophe Fergeau]

From: Frediano Ziglio <fziglio at redhat.com>

Total message size received from the client was unlimited. There is
a 2kiB size check on individual agent messages, but the MonitorsConfig
message can be split in multiple chunks, and the size of the
non-chunked MonitorsConfig message was never checked. This could easily
lead to memory exhaustion on the host.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/reds.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 4aeac9a59..ec2b6f475 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1105,14 +1105,29 @@ void reds_release_agent_data_buffer(RedsState *reds, uint8_t *buf)
 static void reds_on_main_agent_monitors_config(RedsState *reds,
         MainChannelClient *mcc, const void *message, size_t size)
 {
+    const unsigned int MAX_MONITORS = 256;
+    const unsigned int MAX_MONITOR_CONFIG_SIZE =
+       sizeof(VDAgentMonitorsConfig) + MAX_MONITORS * sizeof(VDAgentMonConfig);
+
     VDAgentMessage *msg_header;
     VDAgentMonitorsConfig *monitors_config;
     SpiceBuffer *cmc = &reds->client_monitors_config;
 
+    // limit size of message sent by the client as this can cause a DoS through
+    // memory exhaustion, or potentially some integer overflows
+    if (sizeof(VDAgentMessage) + MAX_MONITOR_CONFIG_SIZE - cmc->offset < size) {
+        goto overflow;
+    }
     spice_buffer_append(cmc, message, size);
+    if (sizeof(VDAgentMessage) > cmc->offset) {
+        spice_debug("not enough data yet. %zd", cmc->offset);
+        return;
+    }
     msg_header = (VDAgentMessage *)cmc->buffer;
-    if (sizeof(VDAgentMessage) > cmc->offset ||
-            msg_header->size > cmc->offset - sizeof(VDAgentMessage)) {
+    if (msg_header->size > MAX_MONITOR_CONFIG_SIZE) {
+        goto overflow;
+    }
+    if (msg_header->size > cmc->offset - sizeof(VDAgentMessage)) {
         spice_debug("not enough data yet. %zd", cmc->offset);
         return;
     }
@@ -1120,6 +1135,12 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
     spice_debug("monitors_config->num_of_monitors: %d", monitors_config->num_of_monitors);
     reds_client_monitors_config(reds, monitors_config);
     spice_buffer_free(cmc);
+    return;
+
+overflow:
+    spice_warning("received invalid MonitorsConfig request from client, disconnecting");
+    red_channel_client_disconnect(RED_CHANNEL_CLIENT(mcc));
+    spice_buffer_free(cmc);
 }
 
 void reds_on_main_agent_data(RedsState *reds, MainChannelClient *mcc, const void *message,
-- 
2.13.0