[Qemu-devel] [PULL 0/1] vnc: fix memory corruption (CVE-2015-5225)

[Qemu-devel] [PULL 0/1] vnc: fix memory corruption (CVE-2015-5225)

[Gerd Hoffmann]

  Hi,

Here comes a vnc security fix.

please pull,
  Gerd

The following changes since commit 7df9671989c1cfa693764f9ae6349324b2ada02a:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20150825-1' into staging (2015-08-25 16:24:06 +0100)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-cve-2015-5225-20150826-1

for you to fetch changes up to eb8934b0418b3b1d125edddc4fc334a54334a49b:

  vnc: fix memory corruption (CVE-2015-5225) (2015-08-26 17:54:33 +0200)

----------------------------------------------------------------
vnc: fix memory corruption (CVE-2015-5225)

----------------------------------------------------------------
Gerd Hoffmann (1):
      vnc: fix memory corruption (CVE-2015-5225)

 ui/vnc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

[Qemu-devel] [PULL 1/1] vnc: fix memory corruption (CVE-2015-5225)

[Gerd Hoffmann]

The _cmp_bytes variable added by commit "bea60dd ui/vnc: fix potential
memory corruption issues" can become negative.  Result is (possibly
exploitable) memory corruption.  Reason for that is it uses the stride
instead of bytes per scanline to apply limits.

For the server surface is is actually fine.  vnc creates that itself,
there is never any padding and thus scanline length always equals stride.

For the guest surface scanline length and stride are typically identical
too, but it doesn't has to be that way.  So add and use a new variable
(guest_ll) for the guest scanline length.  Also rename min_stride to
line_bytes to make more clear what it actually is.  Finally sprinkle
in an assert() to make sure we never use a negative _cmp_bytes again.

Reported-by: 范祚至(库特) <zuozhi.fzz@alibaba-inc.com>
Reviewed-by: P J P <ppandit@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index e26973a..caf82f5 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2872,7 +2872,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
                     pixman_image_get_width(vd->server));
     int height = MIN(pixman_image_get_height(vd->guest.fb),
                      pixman_image_get_height(vd->server));
-    int cmp_bytes, server_stride, min_stride, guest_stride, y = 0;
+    int cmp_bytes, server_stride, line_bytes, guest_ll, guest_stride, y = 0;
     uint8_t *guest_row0 = NULL, *server_row0;
     VncState *vs;
     int has_dirty = 0;
@@ -2891,17 +2891,21 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
      * Update server dirty map.
      */
     server_row0 = (uint8_t *)pixman_image_get_data(vd->server);
-    server_stride = guest_stride = pixman_image_get_stride(vd->server);
+    server_stride = guest_stride = guest_ll =
+        pixman_image_get_stride(vd->server);
     cmp_bytes = MIN(VNC_DIRTY_PIXELS_PER_BIT * VNC_SERVER_FB_BYTES,
                     server_stride);
     if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
         int width = pixman_image_get_width(vd->server);
         tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width);
     } else {
+        int guest_bpp =
+            PIXMAN_FORMAT_BPP(pixman_image_get_format(vd->guest.fb));
         guest_row0 = (uint8_t *)pixman_image_get_data(vd->guest.fb);
         guest_stride = pixman_image_get_stride(vd->guest.fb);
+        guest_ll = pixman_image_get_width(vd->guest.fb) * ((guest_bpp + 7) / 8);
     }
-    min_stride = MIN(server_stride, guest_stride);
+    line_bytes = MIN(server_stride, guest_ll);
 
     for (;;) {
         int x;
@@ -2932,9 +2936,10 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
             if (!test_and_clear_bit(x, vd->guest.dirty[y])) {
                 continue;
             }
-            if ((x + 1) * cmp_bytes > min_stride) {
-                _cmp_bytes = min_stride - x * cmp_bytes;
+            if ((x + 1) * cmp_bytes > line_bytes) {
+                _cmp_bytes = line_bytes - x * cmp_bytes;
             }
+            assert(_cmp_bytes >= 0);
             if (memcmp(server_ptr, guest_ptr, _cmp_bytes) == 0) {
                 continue;
             }
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 0/1] vnc: fix memory corruption (CVE-2015-5225)

[Peter Maydell]

On 26 August 2015 at 17:40, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Here comes a vnc security fix.
>
> please pull,
>   Gerd
>
> The following changes since commit 7df9671989c1cfa693764f9ae6349324b2ada02a:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20150825-1' into staging (2015-08-25 16:24:06 +0100)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-cve-2015-5225-20150826-1
>
> for you to fetch changes up to eb8934b0418b3b1d125edddc4fc334a54334a49b:
>
>   vnc: fix memory corruption (CVE-2015-5225) (2015-08-26 17:54:33 +0200)

So, are we planning to take the "make a stable release for CVE fixes"
approach we discussed at the QEMU summit for this?

(I'm currently applying it to master.)

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/1] vnc: fix memory corruption (CVE-2015-5225)

[Peter Maydell]

On 26 August 2015 at 17:40, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Here comes a vnc security fix.
>
> please pull,
>   Gerd
>
> The following changes since commit 7df9671989c1cfa693764f9ae6349324b2ada02a:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20150825-1' into staging (2015-08-25 16:24:06 +0100)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-cve-2015-5225-20150826-1
>
> for you to fetch changes up to eb8934b0418b3b1d125edddc4fc334a54334a49b:
>
>   vnc: fix memory corruption (CVE-2015-5225) (2015-08-26 17:54:33 +0200)
>
> ----------------------------------------------------------------
> vnc: fix memory corruption (CVE-2015-5225)
>
> ----------------------------------------------------------------
> Gerd Hoffmann (1):
>       vnc: fix memory corruption (CVE-2015-5225)

Applied, thanks.

-- PMM