diff options
author | Frediano Ziglio <fziglio@redhat.com> | 2015-09-09 12:42:09 +0100 |
---|---|---|
committer | Frediano Ziglio <fziglio@redhat.com> | 2015-10-06 11:07:15 +0100 |
commit | dd558bb833254fb49069eca052b92ae1abe3e8ff (patch) | |
tree | 60e77cd6be2b475c3368cd7c74dcb90628ae5774 /server | |
parent | f2ea57335e9d1b37e0747c3b4a860aca0828124e (diff) | |
download | spice-dd558bb833254fb49069eca052b92ae1abe3e8ff.tar.gz spice-dd558bb833254fb49069eca052b92ae1abe3e8ff.tar.xz spice-dd558bb833254fb49069eca052b92ae1abe3e8ff.zip |
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 <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Diffstat (limited to 'server')
-rw-r--r-- | server/red_worker.c | 33 |
1 files changed, 18 insertions, 15 deletions
diff --git a/server/red_worker.c b/server/red_worker.c index d8a081e5..7a60cd12 100644 --- a/server/red_worker.c +++ b/server/red_worker.c @@ -1051,6 +1051,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 @@ -1270,11 +1271,6 @@ static inline int is_primary_surface(RedWorker *worker, uint32_t surface_id) return FALSE; } -static inline void __validate_surface(RedWorker *worker, uint32_t surface_id) -{ - spice_warn_if(surface_id >= worker->n_surfaces); -} - static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable) { DrawContext *context; @@ -1283,7 +1279,7 @@ static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable) /* surface_id must be validated before calling into * validate_drawable_bbox */ - __validate_surface(worker, surface_id); + VALIDATE_SURFACE_RETVAL(worker, surface_id, FALSE); context = &worker->surfaces[surface_id].context; if (drawable->bbox.top < 0) @@ -1304,7 +1300,10 @@ static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable) 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); @@ -4262,12 +4261,14 @@ static inline void red_create_surface(RedWorker *worker, uint32_t surface_id,uin 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]; @@ -4303,6 +4304,7 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface default: spice_error("unknown surface command"); }; +exit: red_put_surface_cmd(surface); free(surface); } @@ -11036,7 +11038,7 @@ void handle_dev_update(void *opaque, void *payload) { 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; @@ -11044,17 +11046,16 @@ void handle_dev_update(void *opaque, void *payload) 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, @@ -11093,6 +11094,7 @@ void handle_dev_del_memslot(void *opaque, void *payload) * 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; } @@ -11362,6 +11364,7 @@ void handle_dev_create_primary_surface(void *opaque, void *payload) 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); |