[Spice-devel] [spice-server 3/3] reds: Avoid buffer overflows handling monitor configuration

[Christophe Fergeau]

From: Frediano Ziglio <fziglio at redhat.com>

It was also possible for a malicious client to set
VDAgentMonitorsConfig::num_of_monitors to a number larger
than the actual size of VDAgentMOnitorsConfig::monitors.
This would lead to buffer overflows, which could allow the guest to
read part of the host memory. This might cause write overflows in the
host as well, but controlling the content of such buffers seems
complicated.

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

diff --git a/server/reds.c b/server/reds.c
index 656f518ff..034cd10d2 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1112,6 +1112,7 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
     VDAgentMessage *msg_header;
     VDAgentMonitorsConfig *monitors_config;
     SpiceBuffer *cmc = &reds->client_monitors_config;
+    uint32_t max_monitors;
 
     // limit size of message sent by the client as this can cause a DoS through
     // memory exhaustion, or potentially some integer overflows
@@ -1135,6 +1136,12 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
         goto overflow;
     }
     monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + sizeof(*msg_header));
+    // limit the monitor number to avoid buffer overflows
+    max_monitors = (msg_header->size - sizeof(VDAgentMonitorsConfig)) /
+                   sizeof(VDAgentMonConfig);
+    if (monitors_config->num_of_monitors > max_monitors) {
+        goto overflow;
+    }
     spice_debug("monitors_config->num_of_monitors: %d", monitors_config->num_of_monitors);
     reds_client_monitors_config(reds, monitors_config);
     spice_buffer_free(cmc);
-- 
2.13.0