|
|
e2c81d |
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
73b8f2 |
From: Frediano Ziglio <fziglio@redhat.com>
|
|
|
73b8f2 |
Date: Wed, 9 Sep 2015 12:42:09 +0100
|
|
|
e2c81d |
Subject: [PATCH] worker: validate correctly surfaces
|
|
|
73b8f2 |
|
|
|
73b8f2 |
Do not just give warning and continue to use an invalid index into
|
|
|
73b8f2 |
an array.
|
|
|
73b8f2 |
|
|
|
73b8f2 |
Resolves: CVE-2015-5260
|
|
|
73b8f2 |
|
|
|
73b8f2 |
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
|
|
|
73b8f2 |
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
|
|
|
73b8f2 |
---
|
|
|
73b8f2 |
server/red_worker.c | 33 ++++++++++++++++++---------------
|
|
|
73b8f2 |
1 file changed, 18 insertions(+), 15 deletions(-)
|
|
|
73b8f2 |
|
|
|
73b8f2 |
diff --git a/server/red_worker.c b/server/red_worker.c
|
|
|
73b8f2 |
index 93e3398..c62dbcb 100644
|
|
|
73b8f2 |
--- a/server/red_worker.c
|
|
|
73b8f2 |
+++ b/server/red_worker.c
|
|
|
73b8f2 |
@@ -1054,6 +1054,7 @@ typedef struct BitmapData {
|
|
|
73b8f2 |
SpiceRect lossy_rect;
|
|
|
73b8f2 |
} BitmapData;
|
|
|
73b8f2 |
|
|
|
73b8f2 |
+static inline int validate_surface(RedWorker *worker, uint32_t surface_id);
|
|
|
73b8f2 |
static void red_draw_qxl_drawable(RedWorker *worker, Drawable *drawable);
|
|
|
73b8f2 |
static void red_current_flush(RedWorker *worker, int surface_id);
|
|
|
73b8f2 |
#ifdef DRAW_ALL
|
|
|
73b8f2 |
@@ -1267,11 +1268,6 @@ static inline int is_primary_surface(RedWorker *worker, uint32_t surface_id)
|
|
|
73b8f2 |
return FALSE;
|
|
|
73b8f2 |
}
|
|
|
73b8f2 |
|
|
|
73b8f2 |
-static inline void __validate_surface(RedWorker *worker, uint32_t surface_id)
|
|
|
73b8f2 |
-{
|
|
|
73b8f2 |
- spice_warn_if(surface_id >= worker->n_surfaces);
|
|
|
73b8f2 |
-}
|
|
|
73b8f2 |
-
|
|
|
73b8f2 |
static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable)
|
|
|
73b8f2 |
{
|
|
|
73b8f2 |
DrawContext *context;
|
|
|
73b8f2 |
@@ -1280,7 +1276,7 @@ static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable)
|
|
|
73b8f2 |
/* surface_id must be validated before calling into
|
|
|
73b8f2 |
* validate_drawable_bbox
|
|
|
73b8f2 |
*/
|
|
|
73b8f2 |
- __validate_surface(worker, surface_id);
|
|
|
73b8f2 |
+ VALIDATE_SURFACE_RETVAL(worker, surface_id, FALSE);
|
|
|
73b8f2 |
context = &worker->surfaces[surface_id].context;
|
|
|
73b8f2 |
|
|
|
73b8f2 |
if (drawable->bbox.top < 0)
|
|
|
73b8f2 |
@@ -1301,7 +1297,10 @@ static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable)
|
|
|
73b8f2 |
|
|
|
73b8f2 |
static inline int validate_surface(RedWorker *worker, uint32_t surface_id)
|
|
|
73b8f2 |
{
|
|
|
73b8f2 |
- spice_warn_if(surface_id >= worker->n_surfaces);
|
|
|
73b8f2 |
+ if SPICE_UNLIKELY(surface_id >= worker->n_surfaces) {
|
|
|
73b8f2 |
+ spice_warning("invalid surface_id %u", surface_id);
|
|
|
73b8f2 |
+ return 0;
|
|
|
73b8f2 |
+ }
|
|
|
73b8f2 |
if (!worker->surfaces[surface_id].context.canvas) {
|
|
|
73b8f2 |
spice_warning("canvas address is %p for %d (and is NULL)\n",
|
|
|
73b8f2 |
&(worker->surfaces[surface_id].context.canvas), surface_id);
|
|
|
73b8f2 |
@@ -4304,12 +4303,14 @@ static inline void red_create_surface(RedWorker *worker, uint32_t surface_id,uin
|
|
|
73b8f2 |
static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface,
|
|
|
73b8f2 |
uint32_t group_id, int loadvm)
|
|
|
73b8f2 |
{
|
|
|
73b8f2 |
- int surface_id;
|
|
|
73b8f2 |
+ uint32_t surface_id;
|
|
|
73b8f2 |
RedSurface *red_surface;
|
|
|
73b8f2 |
uint8_t *data;
|
|
|
73b8f2 |
|
|
|
73b8f2 |
surface_id = surface->surface_id;
|
|
|
73b8f2 |
- __validate_surface(worker, surface_id);
|
|
|
73b8f2 |
+ if SPICE_UNLIKELY(surface_id >= worker->n_surfaces) {
|
|
|
73b8f2 |
+ goto exit;
|
|
|
73b8f2 |
+ }
|
|
|
73b8f2 |
|
|
|
73b8f2 |
red_surface = &worker->surfaces[surface_id];
|
|
|
73b8f2 |
|
|
|
73b8f2 |
@@ -4345,6 +4346,7 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
|
|
|
73b8f2 |
default:
|
|
|
73b8f2 |
spice_error("unknown surface command");
|
|
|
73b8f2 |
};
|
|
|
73b8f2 |
+exit:
|
|
|
73b8f2 |
red_put_surface_cmd(surface);
|
|
|
73b8f2 |
free(surface);
|
|
|
73b8f2 |
}
|
|
|
73b8f2 |
@@ -11091,7 +11093,7 @@ void handle_dev_update(void *opaque, void *payload)
|
|
|
73b8f2 |
{
|
|
|
73b8f2 |
RedWorker *worker = opaque;
|
|
|
73b8f2 |
RedWorkerMessageUpdate *msg = payload;
|
|
|
73b8f2 |
- SpiceRect *rect = spice_new0(SpiceRect, 1);
|
|
|
73b8f2 |
+ SpiceRect *rect;
|
|
|
73b8f2 |
RedSurface *surface;
|
|
|
73b8f2 |
uint32_t surface_id = msg->surface_id;
|
|
|
73b8f2 |
const QXLRect *qxl_area = msg->qxl_area;
|
|
|
73b8f2 |
@@ -11099,17 +11101,16 @@ void handle_dev_update(void *opaque, void *payload)
|
|
|
73b8f2 |
QXLRect *qxl_dirty_rects = msg->qxl_dirty_rects;
|
|
|
73b8f2 |
uint32_t clear_dirty_region = msg->clear_dirty_region;
|
|
|
73b8f2 |
|
|
|
73b8f2 |
+ VALIDATE_SURFACE_RET(worker, surface_id);
|
|
|
73b8f2 |
+
|
|
|
73b8f2 |
+ rect = spice_new0(SpiceRect, 1);
|
|
|
73b8f2 |
surface = &worker->surfaces[surface_id];
|
|
|
73b8f2 |
red_get_rect_ptr(rect, qxl_area);
|
|
|
73b8f2 |
flush_display_commands(worker);
|
|
|
73b8f2 |
|
|
|
73b8f2 |
spice_assert(worker->running);
|
|
|
73b8f2 |
|
|
|
73b8f2 |
- if (validate_surface(worker, surface_id)) {
|
|
|
73b8f2 |
- red_update_area(worker, rect, surface_id);
|
|
|
73b8f2 |
- } else {
|
|
|
73b8f2 |
- rendering_incorrect(__func__);
|
|
|
73b8f2 |
- }
|
|
|
73b8f2 |
+ red_update_area(worker, rect, surface_id);
|
|
|
73b8f2 |
free(rect);
|
|
|
73b8f2 |
|
|
|
73b8f2 |
surface_dirty_region_to_rects(surface, qxl_dirty_rects, num_dirty_rects,
|
|
|
73b8f2 |
@@ -11148,6 +11149,7 @@ void handle_dev_del_memslot(void *opaque, void *payload)
|
|
|
73b8f2 |
* surface_id == 0, maybe move the assert upward and merge the two functions? */
|
|
|
73b8f2 |
static inline void destroy_surface_wait(RedWorker *worker, int surface_id)
|
|
|
73b8f2 |
{
|
|
|
73b8f2 |
+ VALIDATE_SURFACE_RET(worker, surface_id);
|
|
|
73b8f2 |
if (!worker->surfaces[surface_id].context.canvas) {
|
|
|
73b8f2 |
return;
|
|
|
73b8f2 |
}
|
|
|
73b8f2 |
@@ -11412,6 +11414,7 @@ void handle_dev_create_primary_surface(void *opaque, void *payload)
|
|
|
73b8f2 |
|
|
|
73b8f2 |
static void dev_destroy_primary_surface(RedWorker *worker, uint32_t surface_id)
|
|
|
73b8f2 |
{
|
|
|
73b8f2 |
+ VALIDATE_SURFACE_RET(worker, surface_id);
|
|
|
73b8f2 |
spice_warn_if(surface_id != 0);
|
|
|
73b8f2 |
|
|
|
73b8f2 |
spice_debug(NULL);
|