summaryrefslogtreecommitdiffstats
path: root/server
diff options
context:
space:
mode:
authorFrediano Ziglio <fziglio@redhat.com>2015-09-08 12:28:54 +0100
committerFrediano Ziglio <fziglio@redhat.com>2015-10-06 11:11:11 +0100
commit7d69184037d0abb4fcfd5625c765b822aa458808 (patch)
treeff378814428f1942c6d631584e572c5ebef71769 /server
parentf3605979ce3b33d60c33b59334b53618e6d8662a (diff)
downloadspice-7d69184037d0abb4fcfd5625c765b822aa458808.tar.gz
spice-7d69184037d0abb4fcfd5625c765b822aa458808.tar.xz
spice-7d69184037d0abb4fcfd5625c765b822aa458808.zip
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>
Diffstat (limited to 'server')
-rw-r--r--server/red_parse_qxl.c49
1 files changed, 41 insertions, 8 deletions
diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
index f4258697..5513e824 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;
}