| From f1e7ec03e26ab6b8ca9b7ec060846a5b706a963d Mon Sep 17 00:00:00 2001 |
| From: Frediano Ziglio <fziglio@redhat.com> |
| Date: Mon, 15 May 2017 15:57:28 +0100 |
| Subject: [PATCH] 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> |
| Signed-off-by: Peter Korsgaard <peter@korsgaard.com> |
| --- |
| server/reds.c | 25 +++++++++++++++++++++++-- |
| 1 file 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) |
| -- |
| 2.11.0 |
| |