From baffae02e98bfa82977db027959211f7c065009a 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 0fc83608..9e776b91 100644 --- a/server/red_worker.c +++ b/server/red_worker.c @@ -11322,8 +11322,15 @@ static void dev_create_primary_surface(RedWorker *worker, uint32_t surface_id, spice_debug(NULL); spice_warn_if(surface_id != 0); spice_warn_if(surface.height == 0); - spice_warn_if(((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*)get_virt(&worker->mem_slots, surface.mem, surface.height * abs(surface.stride),