From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

From: Frediano Ziglio <fziglio@redhat.com>

Date: Tue, 8 Sep 2015 12:28:54 +0100

Subject: [PATCH] Prevent DoS from guest trying to allocate too much data on

 host for chunks

Limit number of chunks to a given amount to avoid guest trying to

allocate too much memory. Using circular or nested chunks lists

guest could try to allocate huge amounts of memory.

Considering the list can be infinite and guest can change data this

also prevents strange security attacks from guest.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>

---

 server/red_parse_qxl.c | 49 +++++++++++++++++++++++++++++++++++++++++--------

 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c

index fe3ae78..f183248 100644

--- a/server/red_parse_qxl.c

+++ b/server/red_parse_qxl.c

@@ -37,6 +37,13 @@

 G_STATIC_ASSERT(MAX_DATA_CHUNK <= G_MAXINT32);

+/* Limit number of chunks.

+ * The guest can attempt to make host allocate too much memory

+ * just with a large number of small chunks.

+ * Prevent that the chunk list take more memory than the data itself.

+ */

+#define MAX_CHUNKS (MAX_DATA_CHUNK/1024u)

+

 #if 0

 static void hexdump_qxl(RedMemSlotInfo *slots, int group_id,

                         QXLPHYSICAL addr, uint8_t bytes)

@@ -100,9 +107,11 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,

                                       RedDataChunk *red, QXLDataChunk *qxl)

 {

     RedDataChunk *red_prev;

-    size_t data_size = 0;

+    uint64_t data_size = 0;

+    uint32_t chunk_data_size;

     int error;

     QXLPHYSICAL next_chunk;

+    unsigned num_chunks = 0;

     red->data_size = qxl->data_size;

     data_size += red->data_size;

@@ -114,19 +123,43 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,

     }

     while ((next_chunk = qxl->next_chunk) != 0) {

+        /* somebody is trying to use too much memory using a lot of chunks.

+         * Or made a circular list of chunks

+         */

+        if (++num_chunks >= MAX_CHUNKS) {

+            spice_warning("data split in too many chunks, avoiding DoS\n");

+            goto error;

+        }

+

+        memslot_id = get_memslot_id(slots, next_chunk);

+        qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl),

+                                       group_id, &error);

+        if (error)

+            goto error;

+

+        /* do not waste space for empty chunks.

+         * This could be just a driver issue or an attempt

+         * to allocate too much memory or a circular list.

+         * All above cases are handled by the check for number

+         * of chunks.

+         */

+        chunk_data_size = qxl->data_size;

+        if (chunk_data_size == 0)

+            continue;

+

         red_prev = red;

         red = spice_new0(RedDataChunk, 1);

+        red->data_size = chunk_data_size;

         red->prev_chunk = red_prev;

+        red->data = qxl->data;

         red_prev->next_chunk = red;

-        memslot_id = get_memslot_id(slots, next_chunk);

-        qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id,

-                                      &error);

-        if (error)

+        data_size += chunk_data_size;

+        /* this can happen if client is sending nested chunks */

+        if (data_size > MAX_DATA_CHUNK) {

+            spice_warning("too much data inside chunks, avoiding DoS\n");

             goto error;

-        red->data_size = qxl->data_size;

-        data_size += red->data_size;

-        red->data = qxl->data;

+        }

         if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id))

             goto error;

     }