tags 801089 + patch pending tags 801091 + patch pending thanks Dear maintainer,
I've prepared an NMU for spice (versioned as 0.12.5-1.3) and uploaded it to DELAYED/2. Please feel free to tell me if I should delay it longer. Regards, Salvatore
diff -Nru spice-0.12.5/debian/changelog spice-0.12.5/debian/changelog --- spice-0.12.5/debian/changelog 2015-09-05 05:52:55.000000000 +0200 +++ spice-0.12.5/debian/changelog 2015-10-07 18:06:10.000000000 +0200 @@ -1,3 +1,14 @@ +spice (0.12.5-1.3) unstable; urgency=high + + * Non-maintainer upload. + * Add series of patches for CVE-2015-5260 and CVE-2015-6261. + CVE-2015-5260: insufficient validation of surface_id parameter can cause + crash. (Closes: #801089) + CVE-2015-5261: host memory access from guest using crafted images. + (Closes: #801091) + + -- Salvatore Bonaccorso <[email protected]> Wed, 07 Oct 2015 07:23:38 +0200 + spice (0.12.5-1.2) unstable; urgency=high * Non-maintainer upload. diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0001-worker-validate-correctly-surfaces.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0001-worker-validate-correctly-surfaces.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0001-worker-validate-correctly-surfaces.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0001-worker-validate-correctly-surfaces.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,117 @@ +From dd558bb833254fb49069eca052b92ae1abe3e8ff Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Wed, 9 Sep 2015 12:42:09 +0100 +Subject: [PATCH 01/19] worker: validate correctly surfaces + +Do not just give warning and continue to use an invalid index into +an array. + +Resolves: CVE-2015-5260 + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_worker.c | 33 ++++++++++++++++++--------------- + 1 file changed, 18 insertions(+), 15 deletions(-) + +--- a/server/red_worker.c ++++ b/server/red_worker.c +@@ -1036,6 +1036,7 @@ typedef struct BitmapData { + SpiceRect lossy_rect; + } BitmapData; + ++static inline int validate_surface(RedWorker *worker, uint32_t surface_id); + static void red_draw_qxl_drawable(RedWorker *worker, Drawable *drawable); + static void red_current_flush(RedWorker *worker, int surface_id); + #ifdef DRAW_ALL +@@ -1245,14 +1246,12 @@ static inline int is_primary_surface(Red + return FALSE; + } + +-static inline void __validate_surface(RedWorker *worker, uint32_t surface_id) +-{ +- spice_warn_if(surface_id >= worker->n_surfaces); +-} +- + static inline int validate_surface(RedWorker *worker, uint32_t surface_id) + { +- spice_warn_if(surface_id >= worker->n_surfaces); ++ if SPICE_UNLIKELY(surface_id >= worker->n_surfaces) { ++ spice_warning("invalid surface_id %u", surface_id); ++ return 0; ++ } + if (!worker->surfaces[surface_id].context.canvas) { + spice_warning("canvas address is %p for %d (and is NULL)\n", + &(worker->surfaces[surface_id].context.canvas), surface_id); +@@ -4230,12 +4229,14 @@ static inline void red_create_surface(Re + static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface, + uint32_t group_id, int loadvm) + { +- int surface_id; ++ uint32_t surface_id; + RedSurface *red_surface; + uint8_t *data; + + surface_id = surface->surface_id; +- __validate_surface(worker, surface_id); ++ if SPICE_UNLIKELY(surface_id >= worker->n_surfaces) { ++ goto exit; ++ } + + red_surface = &worker->surfaces[surface_id]; + +@@ -4271,6 +4272,7 @@ static inline void red_process_surface(R + default: + spice_error("unknown surface command"); + }; ++exit: + red_put_surface_cmd(surface); + free(surface); + } +@@ -10865,7 +10867,7 @@ void handle_dev_update(void *opaque, voi + { + RedWorker *worker = opaque; + RedWorkerMessageUpdate *msg = payload; +- SpiceRect *rect = spice_new0(SpiceRect, 1); ++ SpiceRect *rect; + RedSurface *surface; + uint32_t surface_id = msg->surface_id; + const QXLRect *qxl_area = msg->qxl_area; +@@ -10873,17 +10875,16 @@ void handle_dev_update(void *opaque, voi + QXLRect *qxl_dirty_rects = msg->qxl_dirty_rects; + uint32_t clear_dirty_region = msg->clear_dirty_region; + ++ VALIDATE_SURFACE_RET(worker, surface_id); ++ ++ rect = spice_new0(SpiceRect, 1); + surface = &worker->surfaces[surface_id]; + red_get_rect_ptr(rect, qxl_area); + flush_display_commands(worker); + + spice_assert(worker->running); + +- if (validate_surface(worker, surface_id)) { +- red_update_area(worker, rect, surface_id); +- } else { +- rendering_incorrect(__func__); +- } ++ red_update_area(worker, rect, surface_id); + free(rect); + + surface_dirty_region_to_rects(surface, qxl_dirty_rects, num_dirty_rects, +@@ -10922,6 +10923,7 @@ void handle_dev_del_memslot(void *opaque + * surface_id == 0, maybe move the assert upward and merge the two functions? */ + static inline void destroy_surface_wait(RedWorker *worker, int surface_id) + { ++ VALIDATE_SURFACE_RET(worker, surface_id); + if (!worker->surfaces[surface_id].context.canvas) { + return; + } +@@ -11186,6 +11188,7 @@ void handle_dev_create_primary_surface(v + + static void dev_destroy_primary_surface(RedWorker *worker, uint32_t surface_id) + { ++ VALIDATE_SURFACE_RET(worker, surface_id); + spice_warn_if(surface_id != 0); + + spice_debug(NULL); diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0002-worker-avoid-double-free-or-double-create-of-surface.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0002-worker-avoid-double-free-or-double-create-of-surface.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0002-worker-avoid-double-free-or-double-create-of-surface.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0002-worker-avoid-double-free-or-double-create-of-surface.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,41 @@ +From 097c638b121e595d9daf79285c447088027a58e2 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Wed, 9 Sep 2015 12:45:06 +0100 +Subject: [PATCH 02/19] worker: avoid double free or double create of surfaces + +A driver can overwrite surface state creating a surface with the same +id of a previous one. +Also can try to destroy surfaces that are not created. +Both requests cause invalid internal states that could lead to crashes +or memory corruptions. + +Signed-off-by: Frediano Ziglio <[email protected]> +--- + server/red_worker.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +--- a/server/red_worker.c ++++ b/server/red_worker.c +@@ -4246,6 +4246,10 @@ static inline void red_process_surface(R + int32_t stride = surface->u.surface_create.stride; + int reloaded_surface = loadvm || (surface->flags & QXL_SURF_FLAG_KEEP_DATA); + ++ if (red_surface->refs) { ++ spice_warning("avoiding creating a surface twice"); ++ break; ++ } + data = surface->u.surface_create.data; + if (stride < 0) { + data -= (int32_t)(stride * (height - 1)); +@@ -4259,7 +4263,10 @@ static inline void red_process_surface(R + break; + } + case QXL_SURFACE_CMD_DESTROY: +- spice_warn_if(!red_surface->context.canvas); ++ if (!red_surface->refs) { ++ spice_warning("avoiding destroying a surface twice"); ++ break; ++ } + set_surface_release_info(worker, surface_id, 0, surface->release_info, group_id); + red_handle_depends_on_target_surface(worker, surface_id); + /* note that red_handle_depends_on_target_surface must be called before red_current_clear. diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0003-Define-a-constant-to-limit-data-from-guest.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0003-Define-a-constant-to-limit-data-from-guest.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0003-Define-a-constant-to-limit-data-from-guest.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0003-Define-a-constant-to-limit-data-from-guest.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,42 @@ +From 0205a6ce63f50af9eda03f14d93b3a2517c42fae Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 11:58:11 +0100 +Subject: [PATCH 03/19] Define a constant to limit data from guest. + +This limit will prevent guest trying to do nasty things and DoS to host. + +Signed-off-by: Frediano Ziglio <[email protected]> +--- + server/red_parse_qxl.c | 11 +++++++++++ + 1 file changed, 11 insertions(+) + +diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c +index 5b1befa..3ffa57b 100644 +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -21,11 +21,22 @@ + + #include <stdbool.h> + #include <inttypes.h> ++#include <glib.h> + #include "common/lz_common.h" + #include "red_common.h" + #include "red_memslots.h" + #include "red_parse_qxl.h" + ++/* Max size in bytes for any data field used in a QXL command. ++ * This will for example be useful to prevent the guest from saturating the ++ * host memory if it tries to send overlapping chunks. ++ * This value should be big enough for all requests but limited ++ * to 32 bits. Even better if it fits on 31 bits to detect integer overflows. ++ */ ++#define MAX_DATA_CHUNK 0x7ffffffflu ++ ++G_STATIC_ASSERT(MAX_DATA_CHUNK <= G_MAXINT32); ++ + #if 0 + static void hexdump_qxl(RedMemSlotInfo *slots, int group_id, + QXLPHYSICAL addr, uint8_t bytes) +-- +2.6.1 + diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0004-Fix-some-integer-overflow-causing-large-memory-alloc.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0004-Fix-some-integer-overflow-causing-large-memory-alloc.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0004-Fix-some-integer-overflow-causing-large-memory-alloc.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0004-Fix-some-integer-overflow-causing-large-memory-alloc.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,64 @@ +From ac5f64a80ae637742ed95fd6c98f66281b3e15c6 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Thu, 17 Sep 2015 15:00:22 +0100 +Subject: [PATCH 04/19] 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 <[email protected]> +--- + server/red_parse_qxl.c | 15 +++++++++++---- + 1 file changed, 11 insertions(+), 4 deletions(-) + +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -384,7 +384,7 @@ static SpiceImage *red_get_image(RedMemS + 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(RedMemS + 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 * abs(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, +@@ -1221,7 +1224,7 @@ int red_get_surface_cmd(RedMemSlotInfo * + RedSurfaceCmd *red, QXLPHYSICAL addr) + { + QXLSurfaceCmd *qxl; +- size_t size; ++ uint64_t size; + int error; + + qxl = (QXLSurfaceCmd *)get_virt(slots, addr, sizeof(*qxl), group_id, +@@ -1241,7 +1244,11 @@ int red_get_surface_cmd(RedMemSlotInfo * + 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) { diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0005-Check-properly-surface-to-be-created.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0005-Check-properly-surface-to-be-created.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0005-Check-properly-surface-to-be-created.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0005-Check-properly-surface-to-be-created.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,73 @@ +From 1eb93baa3c594e1214b1c92bbad8a06e9c7e2d12 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 16:02:59 +0100 +Subject: [PATCH 05/19] Check properly surface to be created + +Check format is valid. +Check stride is at least the size of required bytes for a row. + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 35 ++++++++++++++++++++++++++++++++++- + 1 file changed, 34 insertions(+), 1 deletion(-) + +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -1220,12 +1220,30 @@ void red_put_message(RedMessage *red) + /* nothing yet */ + } + ++static unsigned int surface_format_to_bpp(uint32_t format) ++{ ++ switch (format) { ++ case SPICE_SURFACE_FMT_1_A: ++ return 1; ++ case SPICE_SURFACE_FMT_8_A: ++ return 8; ++ case SPICE_SURFACE_FMT_16_555: ++ case SPICE_SURFACE_FMT_16_565: ++ return 16; ++ case SPICE_SURFACE_FMT_32_xRGB: ++ case SPICE_SURFACE_FMT_32_ARGB: ++ return 32; ++ } ++ return 0; ++} ++ + int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id, + RedSurfaceCmd *red, QXLPHYSICAL addr) + { + QXLSurfaceCmd *qxl; + uint64_t size; + int error; ++ unsigned int bpp; + + qxl = (QXLSurfaceCmd *)get_virt(slots, addr, sizeof(*qxl), group_id, + &error); +@@ -1244,9 +1262,24 @@ int red_get_surface_cmd(RedMemSlotInfo * + 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; ++ bpp = surface_format_to_bpp(red->u.surface_create.format); ++ ++ /* check if format is valid */ ++ if (!bpp) { ++ return 1; ++ } ++ ++ /* check stride is larger than required bytes */ ++ size = ((uint64_t) red->u.surface_create.width * bpp + 7u) / 8u; ++ /* the uint32_t conversion is here to avoid problems with -2^31 value */ ++ if (red->u.surface_create.stride == G_MININT32 ++ || size > (uint32_t) abs(red->u.surface_create.stride)) { ++ return 1; ++ } ++ + /* 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) { ++ if (size > MAX_DATA_CHUNK) { + return 1; + } + red->u.surface_create.data = diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0006-Fix-buffer-reading-overflow.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0006-Fix-buffer-reading-overflow.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0006-Fix-buffer-reading-overflow.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0006-Fix-buffer-reading-overflow.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,38 @@ +From 68a742aaa8d692940ac15d021799b702412887e5 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 10:00:37 +0100 +Subject: [PATCH 06/19] Fix buffer reading overflow + +Not security risk as just for read. +However, this could be used to attempt integer overflows in the +following lines. + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c +index bdd5917..e2f95e4 100644 +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -361,7 +361,14 @@ static const int MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[] = {0, 1, 1, 4, 4, 8, 16, 24, + + static int bitmap_consistent(SpiceBitmap *bitmap) + { +- int bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format]; ++ int bpp; ++ ++ if (bitmap->format >= SPICE_N_ELEMENTS(MAP_BITMAP_FMT_TO_BITS_PER_PIXEL)) { ++ spice_warning("wrong format specified for image\n"); ++ return FALSE; ++ } ++ ++ bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format]; + + if (bitmap->stride < ((bitmap->x * bpp + 7) / 8)) { + spice_warning("image stride too small for width: %d < ((%d * %d + 7) / 8) (%s=%d)\n", +-- +2.6.1 + diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0007-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0007-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0007-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0007-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,46 @@ +From 0f58e9da56e0cbbe4349eefcbb300b6f285e0423 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 13:09:35 +0100 +Subject: [PATCH 07/19] Prevent 32 bit integer overflow in bitmap_consistent + +The overflow may lead to buffer overflow as the row size computed from +width (bitmap->x) can be bigger than the size in bytes (bitmap->stride). +This can make spice-server accept the invalid sizes. + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 7 ++++--- + 1 file changed, 4 insertions(+), 3 deletions(-) + +diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c +index e2f95e4..40c1c99 100644 +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -357,11 +357,12 @@ static const char *bitmap_format_to_string(int format) + return "unknown"; + } + +-static const int MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[] = {0, 1, 1, 4, 4, 8, 16, 24, 32, 32, 8}; ++static const unsigned int MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[] = ++ {0, 1, 1, 4, 4, 8, 16, 24, 32, 32, 8}; + + static int bitmap_consistent(SpiceBitmap *bitmap) + { +- int bpp; ++ unsigned int bpp; + + if (bitmap->format >= SPICE_N_ELEMENTS(MAP_BITMAP_FMT_TO_BITS_PER_PIXEL)) { + spice_warning("wrong format specified for image\n"); +@@ -370,7 +371,7 @@ static int bitmap_consistent(SpiceBitmap *bitmap) + + bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format]; + +- if (bitmap->stride < ((bitmap->x * bpp + 7) / 8)) { ++ if (bitmap->stride < (((uint64_t) bitmap->x * bpp + 7u) / 8u)) { + spice_warning("image stride too small for width: %d < ((%d * %d + 7) / 8) (%s=%d)\n", + bitmap->stride, bitmap->x, bpp, + bitmap_format_to_string(bitmap->format), +-- +2.6.1 + diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0008-Fix-race-condition-on-red_get_clip_rects.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0008-Fix-race-condition-on-red_get_clip_rects.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0008-Fix-race-condition-on-red_get_clip_rects.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0008-Fix-race-condition-on-red_get_clip_rects.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,42 @@ +From 3dfd1a08286d524a742d51952595fcfb6f0c6f1b Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 10:01:51 +0100 +Subject: [PATCH 08/19] Fix race condition on red_get_clip_rects + +Do not read multiple time an array size that can be changed. + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 8 +++++--- + 1 file changed, 5 insertions(+), 3 deletions(-) + +diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c +index 40c1c99..a9f3ca1 100644 +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -273,6 +273,7 @@ static SpiceClipRects *red_get_clip_rects(RedMemSlotInfo *slots, int group_id, + size_t size; + int i; + int error; ++ uint32_t num_rects; + + qxl = (QXLClipRects *)get_virt(slots, addr, sizeof(*qxl), group_id, &error); + if (error) { +@@ -284,9 +285,10 @@ static SpiceClipRects *red_get_clip_rects(RedMemSlotInfo *slots, int group_id, + data = red_linearize_chunk(&chunks, size, &free_data); + red_put_data_chunks(&chunks); + +- spice_assert(qxl->num_rects * sizeof(QXLRect) == size); +- red = spice_malloc(sizeof(*red) + qxl->num_rects * sizeof(SpiceRect)); +- red->num_rects = qxl->num_rects; ++ num_rects = qxl->num_rects; ++ spice_assert(num_rects * sizeof(QXLRect) == size); ++ red = spice_malloc(sizeof(*red) + num_rects * sizeof(SpiceRect)); ++ red->num_rects = num_rects; + + start = (QXLRect*)data; + for (i = 0; i < red->num_rects; i++) { +-- +2.6.1 + diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0009-Fix-race-in-red_get_image.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0009-Fix-race-in-red_get_image.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0009-Fix-race-in-red_get_image.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0009-Fix-race-in-red_get_image.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,72 @@ +From 9235c84e0fbbf5c19305e82fc1607393b35b74ef Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 10:04:10 +0100 +Subject: [PATCH 09/19] Fix race in red_get_image + +Do not read multiple times data from guest as this could be changed +by other vcpu threads. +This causes races and security problems if these data are used for +buffer allocation or checks. + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 18 ++++++++++-------- + 1 file changed, 10 insertions(+), 8 deletions(-) + +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -397,6 +397,7 @@ static SpiceImage *red_get_image(RedMemS + uint64_t bitmap_size, size; + uint8_t qxl_flags; + int error; ++ QXLPHYSICAL palette; + + if (addr == 0) { + return NULL; +@@ -422,12 +423,16 @@ static SpiceImage *red_get_image(RedMemS + switch (red->descriptor.type) { + case SPICE_IMAGE_TYPE_BITMAP: + red->u.bitmap.format = qxl->bitmap.format; +- if (!bitmap_fmt_is_rgb(qxl->bitmap.format) && !qxl->bitmap.palette && !is_mask) { ++ red->u.bitmap.x = qxl->bitmap.x; ++ red->u.bitmap.y = qxl->bitmap.y; ++ red->u.bitmap.stride = qxl->bitmap.stride; ++ palette = qxl->bitmap.palette; ++ if (!bitmap_fmt_is_rgb(red->u.bitmap.format) && !palette && !is_mask) { + spice_warning("guest error: missing palette on bitmap format=%d\n", + red->u.bitmap.format); + goto error; + } +- if (qxl->bitmap.x == 0 || qxl->bitmap.y == 0) { ++ if (red->u.bitmap.x == 0 || red->u.bitmap.y == 0) { + spice_warning("guest error: zero area bitmap\n"); + goto error; + } +@@ -435,23 +440,20 @@ static SpiceImage *red_get_image(RedMemS + if (qxl_flags & QXL_BITMAP_TOP_DOWN) { + red->u.bitmap.flags = SPICE_BITMAP_FLAGS_TOP_DOWN; + } +- red->u.bitmap.x = qxl->bitmap.x; +- red->u.bitmap.y = qxl->bitmap.y; +- red->u.bitmap.stride = qxl->bitmap.stride; + if (!bitmap_consistent(&red->u.bitmap)) { + goto error; + } +- if (qxl->bitmap.palette) { ++ if (palette) { + QXLPalette *qp; + int i, num_ents; +- qp = (QXLPalette *)get_virt(slots, qxl->bitmap.palette, ++ qp = (QXLPalette *)get_virt(slots, palette, + sizeof(*qp), group_id, &error); + if (error) { + goto error; + } + num_ents = qp->num_ents; + if (!validate_virt(slots, (intptr_t)qp->ents, +- get_memslot_id(slots, qxl->bitmap.palette), ++ get_memslot_id(slots, palette), + num_ents * sizeof(qp->ents[0]), group_id)) { + goto error; + } diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0010-Fix-race-condition-in-red_get_string.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0010-Fix-race-condition-in-red_get_string.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0010-Fix-race-condition-in-red_get_string.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0010-Fix-race-condition-in-red_get_string.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,57 @@ +From dfaedec7890069b35f513e4a8ab4071ca54259ff Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 10:05:20 +0100 +Subject: [PATCH 10/19] Fix race condition in red_get_string + +Do not read multiple time an array size that can be changed. + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 15 +++++++++------ + 1 file changed, 9 insertions(+), 6 deletions(-) + +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -810,6 +810,7 @@ static SpiceString *red_get_string(RedMe + size_t chunk_size, qxl_size, red_size, glyph_size; + int glyphs, bpp = 0, i; + int error; ++ uint16_t qxl_flags, qxl_length; + + qxl = (QXLString *)get_virt(slots, addr, sizeof(*qxl), group_id, &error); + if (error) { +@@ -826,13 +827,15 @@ static SpiceString *red_get_string(RedMe + red_put_data_chunks(&chunks); + + qxl_size = qxl->data_size; ++ qxl_flags = qxl->flags; ++ qxl_length = qxl->length; + spice_assert(chunk_size == qxl_size); + +- if (qxl->flags & SPICE_STRING_FLAGS_RASTER_A1) { ++ if (qxl_flags & SPICE_STRING_FLAGS_RASTER_A1) { + bpp = 1; +- } else if (qxl->flags & SPICE_STRING_FLAGS_RASTER_A4) { ++ } else if (qxl_flags & SPICE_STRING_FLAGS_RASTER_A4) { + bpp = 4; +- } else if (qxl->flags & SPICE_STRING_FLAGS_RASTER_A8) { ++ } else if (qxl_flags & SPICE_STRING_FLAGS_RASTER_A8) { + bpp = 8; + } + spice_assert(bpp != 0); +@@ -849,11 +852,11 @@ static SpiceString *red_get_string(RedMe + start = (QXLRasterGlyph*)(&start->data[glyph_size]); + } + spice_assert(start <= end); +- spice_assert(glyphs == qxl->length); ++ spice_assert(glyphs == qxl_length); + + red = spice_malloc(red_size); +- red->length = qxl->length; +- red->flags = qxl->flags; ++ red->length = qxl_length; ++ red->flags = qxl_flags; + + start = (QXLRasterGlyph*)data; + end = (QXLRasterGlyph*)(data + chunk_size); diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0011-Fix-integer-overflow-computing-glyph_size-in-red_get.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0011-Fix-integer-overflow-computing-glyph_size-in-red_get.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0011-Fix-integer-overflow-computing-glyph_size-in-red_get.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0011-Fix-integer-overflow-computing-glyph_size-in-red_get.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,58 @@ +From caec52dc77af6ebdac3219a1b10fe2293af21208 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 10:13:24 +0100 +Subject: [PATCH 11/19] Fix integer overflow computing glyph_size in + red_get_string + +If bpp is int the formula can lead to weird overflows. width and height +are uint16_t so the formula is: + + size_t = u16 * (u16 * int + const_int) / const_int; + +so it became + + size_t = (int) u16 * ((int) u16 * int + const_int) / const_int; + +However the (int) u16 * (int) u16 can then became negative to overflow. +Under 64 bit architectures size_t is 64 and int usually 32 so converting +this negative 32 bit number to a unsigned 64 bit lead to a very big +number as the signed is extended and then converted to unsigned. +Using unsigned arithmetic prevent extending the sign. + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 8 +++++--- + 1 file changed, 5 insertions(+), 3 deletions(-) + +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -808,7 +808,9 @@ static SpiceString *red_get_string(RedMe + uint8_t *data; + bool free_data; + size_t chunk_size, qxl_size, red_size, glyph_size; +- int glyphs, bpp = 0, i; ++ int glyphs, i; ++ /* use unsigned to prevent integer overflow in multiplication below */ ++ unsigned int bpp = 0; + int error; + uint16_t qxl_flags, qxl_length; + +@@ -847,7 +849,7 @@ static SpiceString *red_get_string(RedMe + while (start < end) { + spice_assert((QXLRasterGlyph*)(&start->data[0]) <= end); + glyphs++; +- glyph_size = start->height * ((start->width * bpp + 7) / 8); ++ glyph_size = start->height * ((start->width * bpp + 7u) / 8u); + red_size += sizeof(SpiceRasterGlyph *) + SPICE_ALIGN(sizeof(SpiceRasterGlyph) + glyph_size, 4); + start = (QXLRasterGlyph*)(&start->data[glyph_size]); + } +@@ -868,7 +870,7 @@ static SpiceString *red_get_string(RedMe + glyph->height = start->height; + red_get_point_ptr(&glyph->render_pos, &start->render_pos); + red_get_point_ptr(&glyph->glyph_origin, &start->glyph_origin); +- glyph_size = glyph->height * ((glyph->width * bpp + 7) / 8); ++ glyph_size = glyph->height * ((glyph->width * bpp + 7u) / 8u); + spice_assert((QXLRasterGlyph*)(&start->data[glyph_size]) <= end); + memcpy(glyph->data, start->data, glyph_size); + start = (QXLRasterGlyph*)(&start->data[glyph_size]); diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0012-Fix-race-condition-in-red_get_data_chunks_ptr.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0012-Fix-race-condition-in-red_get_data_chunks_ptr.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0012-Fix-race-condition-in-red_get_data_chunks_ptr.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0012-Fix-race-condition-in-red_get_data_chunks_ptr.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,66 @@ +From 3738478ed7065fe05f3ee4848f8a7fcdf40aa920 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 12:12:19 +0100 +Subject: [PATCH 12/19] Fix race condition in red_get_data_chunks_ptr + +Do not read multiple times data from guest as this can be changed by +other guest vcpus. This causes races and security problems if these +data are used for buffer allocation or checks. + +Actually, the 'data' member can't change during read as it is just a +pointer to a fixed array contained in qxl. However, this change will +make it clear that there can be no race condition. + +Signed-off-by: Frediano Ziglio <[email protected]> +--- + server/red_parse_qxl.c | 17 ++++++++++------- + 1 file changed, 10 insertions(+), 7 deletions(-) + +diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c +index cfa21f9..2863ae2 100644 +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -102,30 +102,33 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id, + RedDataChunk *red_prev; + size_t data_size = 0; + int error; ++ QXLPHYSICAL next_chunk; + + red->data_size = qxl->data_size; + data_size += red->data_size; +- if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) { ++ red->data = qxl->data; ++ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) { ++ red->data = NULL; + return 0; + } +- red->data = qxl->data; + red->prev_chunk = NULL; + +- while (qxl->next_chunk) { ++ while ((next_chunk = qxl->next_chunk) != 0) { + red_prev = red; + red = spice_new(RedDataChunk, 1); +- memslot_id = get_memslot_id(slots, qxl->next_chunk); +- qxl = (QXLDataChunk *)get_virt(slots, qxl->next_chunk, sizeof(*qxl), group_id, ++ memslot_id = get_memslot_id(slots, next_chunk); ++ qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id, + &error); + if (error) { + return 0; + } + red->data_size = qxl->data_size; + data_size += red->data_size; +- if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) { ++ red->data = qxl->data; ++ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) { ++ red->data = NULL; + return 0; + } +- red->data = qxl->data; + red->prev_chunk = red_prev; + red_prev->next_chunk = red; + } +-- +2.6.1 + diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0013-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0013-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0013-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0013-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,75 @@ +From f3605979ce3b33d60c33b59334b53618e6d8662a Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 12:14:55 +0100 +Subject: [PATCH 13/19] Prevent memory leak if red_get_data_chunks_ptr fails + +Free linked list if client tries to do nasty things + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 31 ++++++++++++++++++++----------- + 1 file changed, 20 insertions(+), 11 deletions(-) + +diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c +index 2863ae2..f425869 100644 +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -107,34 +107,43 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id, + red->data_size = qxl->data_size; + data_size += red->data_size; + red->data = qxl->data; ++ red->prev_chunk = red->next_chunk = NULL; + if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) { + red->data = NULL; + return 0; + } +- red->prev_chunk = NULL; + + while ((next_chunk = qxl->next_chunk) != 0) { + red_prev = red; +- red = spice_new(RedDataChunk, 1); ++ red = spice_new0(RedDataChunk, 1); ++ red->prev_chunk = red_prev; ++ 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) { +- return 0; +- } ++ if (error) ++ 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)) { +- red->data = NULL; +- return 0; +- } +- red->prev_chunk = red_prev; +- red_prev->next_chunk = red; ++ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) ++ goto error; + } + + red->next_chunk = NULL; + return data_size; ++ ++error: ++ while (red->prev_chunk) { ++ red_prev = red->prev_chunk; ++ free(red); ++ red = red_prev; ++ } ++ red->data_size = 0; ++ red->next_chunk = NULL; ++ red->data = NULL; ++ return 0; + } + + static size_t red_get_data_chunks(RedMemSlotInfo *slots, int group_id, +-- +2.6.1 + diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0014-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0014-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0014-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0014-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,102 @@ +From 7d69184037d0abb4fcfd5625c765b822aa458808 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 12:28:54 +0100 +Subject: [PATCH 14/19] 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 <[email protected]> +--- + 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 f425869..5513e82 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; + } +-- +2.6.1 + diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0015-Fix-some-possible-overflows-in-red_get_string-for-32.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0015-Fix-some-possible-overflows-in-red_get_string-for-32.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0015-Fix-some-possible-overflows-in-red_get_string-for-32.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0015-Fix-some-possible-overflows-in-red_get_string-for-32.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,36 @@ +From a447c4f2ac19a1fa36330ffc90ee70b953b82050 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 8 Sep 2015 13:06:03 +0100 +Subject: [PATCH 15/19] Fix some possible overflows in red_get_string for 32 + bit + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 8 +++++++- + 1 file changed, 7 insertions(+), 1 deletion(-) + +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -896,6 +896,11 @@ static SpiceString *red_get_string(RedMe + glyphs++; + glyph_size = start->height * ((start->width * bpp + 7u) / 8u); + red_size += sizeof(SpiceRasterGlyph *) + SPICE_ALIGN(sizeof(SpiceRasterGlyph) + glyph_size, 4); ++ /* do the test correctly, we know end - start->data[0] cannot ++ * overflow, don't use start->data[glyph_size] to test for ++ * buffer overflow as this on 32 bit can cause overflow ++ * on the pointer arithmetic */ ++ spice_assert(glyph_size <= (char*) end - (char*) &start->data[0]); + start = (QXLRasterGlyph*)(&start->data[glyph_size]); + } + spice_assert(start <= end); +@@ -916,7 +921,8 @@ static SpiceString *red_get_string(RedMe + red_get_point_ptr(&glyph->render_pos, &start->render_pos); + red_get_point_ptr(&glyph->glyph_origin, &start->glyph_origin); + glyph_size = glyph->height * ((glyph->width * bpp + 7u) / 8u); +- spice_assert((QXLRasterGlyph*)(&start->data[glyph_size]) <= end); ++ /* see above for similar test */ ++ spice_assert(glyph_size <= (char*) end - (char*) &start->data[0]); + memcpy(glyph->data, start->data, glyph_size); + start = (QXLRasterGlyph*)(&start->data[glyph_size]); + glyph = (SpiceRasterGlyph*) diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0016-Make-sure-we-can-read-QXLPathSeg-structures.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0016-Make-sure-we-can-read-QXLPathSeg-structures.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0016-Make-sure-we-can-read-QXLPathSeg-structures.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0016-Make-sure-we-can-read-QXLPathSeg-structures.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,40 @@ +From 2693e0497e5626642250cff47a59b3b4b2cd432d Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 15 Sep 2015 16:25:17 +0100 +Subject: [PATCH 16/19] Make sure we can read QXLPathSeg structures + +start pointer points to a QXLPathSeg structure. +Before reading from the structure, make sure the structure is contained +in the memory range checked. + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c +index f21bfa5..281faad 100644 +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -256,7 +256,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id, + + start = (QXLPathSeg*)data; + end = (QXLPathSeg*)(data + size); +- while (start < end) { ++ while (start+1 < end) { + n_segments++; + count = start->count; + segment_size = sizeof(SpicePathSeg) + count * sizeof(SpicePointFix); +@@ -272,7 +272,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id, + seg = (SpicePathSeg*)&red->segments[n_segments]; + n_segments = 0; + mem_size2 = sizeof(*red); +- while (start < end) { ++ while (start+1 < end) { + red->segments[n_segments++] = seg; + count = start->count; + +-- +2.6.1 + diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0017-Avoid-race-condition-copying-segments-in-red_get_pat.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0017-Avoid-race-condition-copying-segments-in-red_get_pat.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0017-Avoid-race-condition-copying-segments-in-red_get_pat.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0017-Avoid-race-condition-copying-segments-in-red_get_pat.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,31 @@ +From 2b6695f1222f68690ea230e4e37ded7e07188f06 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Tue, 15 Sep 2015 16:38:23 +0100 +Subject: [PATCH 17/19] Avoid race condition copying segments in red_get_path + +The guest can attempt to increase the number of segments while +spice-server is reading them. +Make sure we don't copy more then the allocated segments. + +Signed-off-by: Frediano Ziglio <[email protected]> +Acked-by: Christophe Fergeau <[email protected]> +--- + server/red_parse_qxl.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c +index 281faad..c7f8650 100644 +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -272,7 +272,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id, + seg = (SpicePathSeg*)&red->segments[n_segments]; + n_segments = 0; + mem_size2 = sizeof(*red); +- while (start+1 < end) { ++ while (start+1 < end && n_segments < red->num_segments) { + red->segments[n_segments++] = seg; + count = start->count; + +-- +2.6.1 + diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0018-Prevent-data_size-to-be-set-independently-from-data.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0018-Prevent-data_size-to-be-set-independently-from-data.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0018-Prevent-data_size-to-be-set-independently-from-data.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0018-Prevent-data_size-to-be-set-independently-from-data.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,24 @@ +From b3be589ab3b32af3e470a9dec19a61fb086f72fc Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Thu, 17 Sep 2015 14:28:36 +0100 +Subject: [PATCH 18/19] Prevent data_size to be set independently from data + +There was not check for data_size field so one could set data to +a small set of data and data_size much bigger than size of data +leading to buffer overflow. + +Signed-off-by: Frediano Ziglio <[email protected]> +--- + server/red_parse_qxl.c | 1 + + 1 file changed, 1 insertion(+) + +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -1392,6 +1392,7 @@ static int red_get_cursor(RedMemSlotInfo + size = red_get_data_chunks_ptr(slots, group_id, + get_memslot_id(slots, addr), + &chunks, &qxl->chunk); ++ red->data_size = MIN(red->data_size, size); + data = red_linearize_chunk(&chunks, size, &free_data); + red_put_data_chunks(&chunks); + if (free_data) { diff -Nru spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0019-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0019-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch --- spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0019-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch 1970-01-01 01:00:00.000000000 +0100 +++ spice-0.12.5/debian/patches/CVE-2015-5260_CVE-2015-5261/0019-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch 2015-10-07 18:06:10.000000000 +0200 @@ -0,0 +1,29 @@ +From 6e3547f8b192f5b01d478ca222bf46736f5c700c Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio <[email protected]> +Date: Thu, 17 Sep 2015 15:01:05 +0100 +Subject: [PATCH 19/19] Prevent leak if size from red_get_data_chunks don't + match in red_get_image + +Signed-off-by: Frediano Ziglio <[email protected]> +--- + server/red_parse_qxl.c | 2 ++ + 1 file changed, 2 insertions(+) + +--- a/server/red_parse_qxl.c ++++ b/server/red_parse_qxl.c +@@ -530,6 +530,7 @@ static SpiceImage *red_get_image(RedMemS + &chunks, qxl->bitmap.data); + spice_assert(size == bitmap_size); + if (size != bitmap_size) { ++ red_put_data_chunks(&chunks); + goto error; + } + red->u.bitmap.data = red_get_image_data_chunked(slots, group_id, +@@ -550,6 +551,7 @@ static SpiceImage *red_get_image(RedMemS + &chunks, (QXLDataChunk *)qxl->quic.data); + spice_assert(size == red->u.quic.data_size); + if (size != red->u.quic.data_size) { ++ red_put_data_chunks(&chunks); + goto error; + } + red->u.quic.data = red_get_image_data_chunked(slots, group_id, diff -Nru spice-0.12.5/debian/patches/series spice-0.12.5/debian/patches/series --- spice-0.12.5/debian/patches/series 2015-09-05 05:52:55.000000000 +0200 +++ spice-0.12.5/debian/patches/series 2015-10-07 18:06:10.000000000 +0200 @@ -1,2 +1,21 @@ fix-tests-warnings.patch CVE-2015-3247.patch +CVE-2015-5260_CVE-2015-5261/0001-worker-validate-correctly-surfaces.patch +CVE-2015-5260_CVE-2015-5261/0002-worker-avoid-double-free-or-double-create-of-surface.patch +CVE-2015-5260_CVE-2015-5261/0003-Define-a-constant-to-limit-data-from-guest.patch +CVE-2015-5260_CVE-2015-5261/0004-Fix-some-integer-overflow-causing-large-memory-alloc.patch +CVE-2015-5260_CVE-2015-5261/0005-Check-properly-surface-to-be-created.patch +CVE-2015-5260_CVE-2015-5261/0006-Fix-buffer-reading-overflow.patch +CVE-2015-5260_CVE-2015-5261/0007-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch +CVE-2015-5260_CVE-2015-5261/0008-Fix-race-condition-on-red_get_clip_rects.patch +CVE-2015-5260_CVE-2015-5261/0009-Fix-race-in-red_get_image.patch +CVE-2015-5260_CVE-2015-5261/0010-Fix-race-condition-in-red_get_string.patch +CVE-2015-5260_CVE-2015-5261/0011-Fix-integer-overflow-computing-glyph_size-in-red_get.patch +CVE-2015-5260_CVE-2015-5261/0012-Fix-race-condition-in-red_get_data_chunks_ptr.patch +CVE-2015-5260_CVE-2015-5261/0013-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch +CVE-2015-5260_CVE-2015-5261/0014-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch +CVE-2015-5260_CVE-2015-5261/0015-Fix-some-possible-overflows-in-red_get_string-for-32.patch +CVE-2015-5260_CVE-2015-5261/0016-Make-sure-we-can-read-QXLPathSeg-structures.patch +CVE-2015-5260_CVE-2015-5261/0017-Avoid-race-condition-copying-segments-in-red_get_pat.patch +CVE-2015-5260_CVE-2015-5261/0018-Prevent-data_size-to-be-set-independently-from-data.patch +CVE-2015-5260_CVE-2015-5261/0019-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch
