[Spice-devel] [PATCH 2/2] improve primary surface parameter checks

[Frediano Ziglio]

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 <fziglio at redhat.com>
Acked-by: Christophe Fergeau <cfergeau at redhat.com>
---
 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 e754bd2..121a2e5 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),
-- 
2.7.4