9ae3a8
From e543257370cce5153bbcf0085a116e6aa4a6d91b Mon Sep 17 00:00:00 2001
9ae3a8
From: Gerd Hoffmann <kraxel@redhat.com>
9ae3a8
Date: Wed, 22 Feb 2017 12:36:25 +0100
9ae3a8
Subject: [PATCH 07/24] vnc: fix memory corruption (CVE-2015-5225)
9ae3a8
MIME-Version: 1.0
9ae3a8
Content-Type: text/plain; charset=UTF-8
9ae3a8
Content-Transfer-Encoding: 8bit
9ae3a8
9ae3a8
RH-Author: Gerd Hoffmann <kraxel@redhat.com>
9ae3a8
Message-id: <1487766986-6329-8-git-send-email-kraxel@redhat.com>
9ae3a8
Patchwork-id: 73978
9ae3a8
O-Subject: [RHEL-7.4 qemu-kvm PATCH 7/8] vnc: fix memory corruption (CVE-2015-5225)
9ae3a8
Bugzilla: 1377977
9ae3a8
RH-Acked-by: Thomas Huth <thuth@redhat.com>
9ae3a8
RH-Acked-by: Marc-André Lureau <mlureau@redhat.com>
9ae3a8
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
9ae3a8
9ae3a8
The _cmp_bytes variable added by commit "bea60dd ui/vnc: fix potential
9ae3a8
memory corruption issues" can become negative.  Result is (possibly
9ae3a8
exploitable) memory corruption.  Reason for that is it uses the stride
9ae3a8
instead of bytes per scanline to apply limits.
9ae3a8
9ae3a8
For the server surface is is actually fine.  vnc creates that itself,
9ae3a8
there is never any padding and thus scanline length always equals stride.
9ae3a8
9ae3a8
For the guest surface scanline length and stride are typically identical
9ae3a8
too, but it doesn't has to be that way.  So add and use a new variable
9ae3a8
(guest_ll) for the guest scanline length.  Also rename min_stride to
9ae3a8
line_bytes to make more clear what it actually is.  Finally sprinkle
9ae3a8
in an assert() to make sure we never use a negative _cmp_bytes again.
9ae3a8
9ae3a8
Reported-by: 范祚至(库特) <zuozhi.fzz@alibaba-inc.com>
9ae3a8
Reviewed-by: P J P <ppandit@redhat.com>
9ae3a8
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
9ae3a8
(cherry picked from commit eb8934b0418b3b1d125edddc4fc334a54334a49b)
9ae3a8
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
9ae3a8
---
9ae3a8
 ui/vnc.c | 15 ++++++++++-----
9ae3a8
 1 file changed, 10 insertions(+), 5 deletions(-)
9ae3a8
9ae3a8
diff --git a/ui/vnc.c b/ui/vnc.c
9ae3a8
index 80b7792..d0ada7e 100644
9ae3a8
--- a/ui/vnc.c
9ae3a8
+++ b/ui/vnc.c
9ae3a8
@@ -2676,7 +2676,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
9ae3a8
                     pixman_image_get_width(vd->server));
9ae3a8
     int height = MIN(pixman_image_get_height(vd->guest.fb),
9ae3a8
                      pixman_image_get_height(vd->server));
9ae3a8
-    int cmp_bytes, server_stride, min_stride, guest_stride, y = 0;
9ae3a8
+    int cmp_bytes, server_stride, line_bytes, guest_ll, guest_stride, y = 0;
9ae3a8
     uint8_t *guest_row0 = NULL, *server_row0;
9ae3a8
     VncState *vs;
9ae3a8
     int has_dirty = 0;
9ae3a8
@@ -2695,17 +2695,21 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
9ae3a8
      * Update server dirty map.
9ae3a8
      */
9ae3a8
     server_row0 = (uint8_t *)pixman_image_get_data(vd->server);
9ae3a8
-    server_stride = guest_stride = pixman_image_get_stride(vd->server);
9ae3a8
+    server_stride = guest_stride = guest_ll =
9ae3a8
+        pixman_image_get_stride(vd->server);
9ae3a8
     cmp_bytes = MIN(VNC_DIRTY_PIXELS_PER_BIT * VNC_SERVER_FB_BYTES,
9ae3a8
                     server_stride);
9ae3a8
     if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
9ae3a8
         int width = pixman_image_get_width(vd->server);
9ae3a8
         tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width);
9ae3a8
     } else {
9ae3a8
+        int guest_bpp =
9ae3a8
+            PIXMAN_FORMAT_BPP(pixman_image_get_format(vd->guest.fb));
9ae3a8
         guest_row0 = (uint8_t *)pixman_image_get_data(vd->guest.fb);
9ae3a8
         guest_stride = pixman_image_get_stride(vd->guest.fb);
9ae3a8
+        guest_ll = pixman_image_get_width(vd->guest.fb) * ((guest_bpp + 7) / 8);
9ae3a8
     }
9ae3a8
-    min_stride = MIN(server_stride, guest_stride);
9ae3a8
+    line_bytes = MIN(server_stride, guest_ll);
9ae3a8
 
9ae3a8
     for (;;) {
9ae3a8
         int x;
9ae3a8
@@ -2736,9 +2740,10 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
9ae3a8
             if (!test_and_clear_bit(x, vd->guest.dirty[y])) {
9ae3a8
                 continue;
9ae3a8
             }
9ae3a8
-            if ((x + 1) * cmp_bytes > min_stride) {
9ae3a8
-                _cmp_bytes = min_stride - x * cmp_bytes;
9ae3a8
+            if ((x + 1) * cmp_bytes > line_bytes) {
9ae3a8
+                _cmp_bytes = line_bytes - x * cmp_bytes;
9ae3a8
             }
9ae3a8
+            assert(_cmp_bytes >= 0);
9ae3a8
             if (memcmp(server_ptr, guest_ptr, _cmp_bytes) == 0) {
9ae3a8
                 continue;
9ae3a8
             }
9ae3a8
-- 
9ae3a8
1.8.3.1
9ae3a8