From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

From: Frediano Ziglio <fziglio@redhat.com>

Date: Thu, 17 Sep 2015 15:00:22 +0100

Subject: [PATCH] Fix some integer overflow causing large memory allocations

Prevent integer overflow when computing image sizes.

Image index computations are done using 32 bit so this can cause easily

security issues. MAX_DATA_CHUNK is larger than the virtual

card limit, so this is not going to cause change in behaviours.

Comparing size calculation results with MAX_DATA_CHUNK will allow us to

catch overflows.

Prevent guest from allocating large amount of memory.

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

---

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

 1 file changed, 11 insertions(+), 4 deletions(-)

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

index 4449f2c..ceb2759 100644

--- a/server/red_parse_qxl.c

+++ b/server/red_parse_qxl.c

@@ -384,7 +384,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,

     QXLImage *qxl;

     SpiceImage *red = NULL;

     SpicePalette *rp = NULL;

-    size_t bitmap_size, size;

+    uint64_t bitmap_size, size;

     uint8_t qxl_flags;

     int error;

@@ -460,7 +460,10 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,

             red->u.bitmap.palette = rp;

             red->u.bitmap.palette_id = rp->unique;

         }

-        bitmap_size = red->u.bitmap.y * abs(red->u.bitmap.stride);

+        bitmap_size = (uint64_t) red->u.bitmap.y * red->u.bitmap.stride;

+        if (bitmap_size > MAX_DATA_CHUNK) {

+            goto error;

+        }

         if (qxl_flags & QXL_BITMAP_DIRECT) {

             red->u.bitmap.data = red_get_image_data_flat(slots, group_id,

                                                          qxl->bitmap.data,

@@ -1220,7 +1223,7 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,

                         RedSurfaceCmd *red, QXLPHYSICAL addr)

 {

     QXLSurfaceCmd *qxl;

-    size_t size;

+    uint64_t size;

     int error;

     qxl = (QXLSurfaceCmd *)get_virt(slots, addr, sizeof(*qxl), group_id,

@@ -1240,7 +1243,11 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,

         red->u.surface_create.width  = qxl->u.surface_create.width;

         red->u.surface_create.height = qxl->u.surface_create.height;

         red->u.surface_create.stride = qxl->u.surface_create.stride;

-        size = red->u.surface_create.height * abs(red->u.surface_create.stride);

+        /* the multiplication can overflow, also abs(-2^31) may return a negative value */

+        size = (uint64_t) red->u.surface_create.height * abs(red->u.surface_create.stride);

+        if (size > MAX_DATA_CHUNK || red->u.surface_create.stride == G_MININT32) {

+            return 1;

+        }

         red->u.surface_create.data =

             (uint8_t*)get_virt(slots, qxl->u.surface_create.data, size, group_id, &error);

         if (error) {