From 69628ea1375282cb7ca5b4dc4410e7aa67e0fc02 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Wed, 2 Mar 2016 12:35:41 +0000 Subject: [PATCH] improve primary surface parameter checks Primary surface, as additional surfaces, can be used to access host memory from the guest using invalid parameters. The removed warning is not enough to prevent all cases. Also a warning is not enough to stop an escalation to happen. The red_validate_surface do different checks to make sure surface request is valid and not cause possible buffer/integer overflows: - format is valid; - width is not large to cause overflow compared to stride; - stride is not -2^31 (a number which negate is still <0); - stride * height does not overflow. This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1312980. Signed-off-by: Frediano Ziglio Acked-by: Christophe Fergeau --- server/red-worker.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/server/red-worker.c b/server/red-worker.c index e754bd2f..121a2e5c 100644 --- a/server/red-worker.c +++ b/server/red-worker.c @@ -657,8 +657,15 @@ static void dev_create_primary_surface(RedWorker *worker, uint32_t surface_id, spice_debug(NULL); spice_warn_if_fail(surface_id == 0); spice_warn_if_fail(surface.height != 0); - spice_warn_if_fail(((uint64_t)abs(surface.stride) * (uint64_t)surface.height) == - abs(surface.stride) * surface.height); + + /* surface can arrive from guest unchecked so make sure + * guest is not a malicious one and drop invalid requests + */ + if (!red_validate_surface(surface.width, surface.height, + surface.stride, surface.format)) { + spice_warning("wrong primary surface creation request"); + return; + } line_0 = (uint8_t*)memslot_get_virt(&worker->mem_slots, surface.mem, surface.height * abs(surface.stride),