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 11:14:04 +0200
commitf1e7ec03e26ab6b8ca9b7ec060846a5b706a963d (patch)
tree8244c4dbba86aed7da43b80464ed351344283496
parentbb79d369e8d0796a5d4f5e578171a1ef807d2ab6 (diff)
reds: Disconnect when receiving overly big ClientMonitorsConfig
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@redhat.com>
-rw-r--r--server/reds.c25
1 files changed, 23 insertions, 2 deletions
diff --git a/server/reds.c b/server/reds.c
index f439a366..7be85fdf 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -993,19 +993,34 @@ static void reds_client_monitors_config_cleanup(void)
static void reds_on_main_agent_monitors_config(
MainChannelClient *mcc, 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;
RedsClientMonitorsConfig *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->buffer_size < size) {
+ goto overflow;
+ }
cmc->buffer_size += size;
cmc->buffer = realloc(cmc->buffer, cmc->buffer_size);
spice_assert(cmc->buffer);
cmc->mcc = mcc;
memcpy(cmc->buffer + cmc->buffer_pos, message, size);
cmc->buffer_pos += size;
+ if (sizeof(VDAgentMessage) > cmc->buffer_size) {
+ spice_debug("not enough data yet. %d", cmc->buffer_size);
+ return;
+ }
msg_header = (VDAgentMessage *)cmc->buffer;
- if (sizeof(VDAgentMessage) > cmc->buffer_size ||
- msg_header->size > cmc->buffer_size - sizeof(VDAgentMessage)) {
+ if (msg_header->size > MAX_MONITOR_CONFIG_SIZE) {
+ goto overflow;
+ }
+ if (msg_header->size > cmc->buffer_size - sizeof(VDAgentMessage)) {
spice_debug("not enough data yet. %d", cmc->buffer_size);
return;
}
@@ -1013,6 +1028,12 @@ static void reds_on_main_agent_monitors_config(
spice_debug("%s: %d", __func__, monitors_config->num_of_monitors);
red_dispatcher_client_monitors_config(monitors_config);
reds_client_monitors_config_cleanup();
+ return;
+
+overflow:
+ spice_warning("received invalid MonitorsConfig request from client, disconnecting");
+ red_channel_client_disconnect(main_channel_client_get_base(mcc));
+ reds_client_monitors_config_cleanup();
}
void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t size)