From b1c167bb9e8047e93bfd43a43832963c8e830f5b Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Wed, 2 Mar 2016 12:35:41 +0000 Subject: [PATCH 2/2] 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 241c300..c7fc8bd 100644 --- a/server/red-worker.c +++ b/server/red-worker.c @@ -681,8 +681,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), -- 2.7.3