summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFrediano Ziglio <fziglio@redhat.com>2017-05-15 15:57:28 +0100
committerChristophe Fergeau <cfergeau@redhat.com>2017-07-11 10:40:27 +0200
commitfbbcdad773e2791cfb988f4748faa41943551ca6 (patch)
treebb93cf41a67347772f97056e025771cc97200ec7
parent571cec91e71c2aae0d5f439ea2d8439d0c3d75eb (diff)
reds: Avoid buffer overflows handling monitor configuration
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@redhat.com>
-rw-r--r--server/reds.c7
1 files changed, 7 insertions, 0 deletions
diff --git a/server/reds.c b/server/reds.c
index 656f518f..034cd10d 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);