[Qemu-devel] [PULL 0/5] vmware-vga: add rectangle verification (CVE-2014-3689)

[Qemu-devel] [PULL 0/5] vmware-vga: add rectangle verification (CVE-2014-3689)

[Gerd Hoffmann]

  Hi,

After three revisions now finally the
pull req for the CVE-2014-3689 fixes.

please pull,
  Gerd

The following changes since commit 3e9418e160cd8901c83a3c88967158084f5b5c03:

  Revert "main-loop.c: Handle SIGINT, SIGHUP and SIGTERM synchronously" (2014-10-27 15:05:09 +0000)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/pull-cve-2014-3689-20141029-1

for you to fetch changes up to bd9ccd8517e83b7c33a9167815dbfffb30d70b13:

  vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect (2014-10-29 12:01:30 +0100)

----------------------------------------------------------------
vmware-vga: add rectangle verification (CVE-2014-3689)

----------------------------------------------------------------
Gerd Hoffmann (5):
      vmware-vga: CVE-2014-3689: turn off hw accel
      vmware-vga: add vmsvga_verify_rect
      vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect
      vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect
      vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect

 hw/display/vmware_vga.c | 116 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 78 insertions(+), 38 deletions(-)

[Qemu-devel] [PULL 1/5] vmware-vga: CVE-2014-3689: turn off hw accel

[Gerd Hoffmann]

Quick & easy stopgap for CVE-2014-3689:  We just compile out the
hardware acceleration functions which lack sanity checks.  Thankfully
we have capability bits for them (SVGA_CAP_RECT_COPY and
SVGA_CAP_RECT_FILL), so guests should deal just fine, in theory.

Subsequent patches will add the missing checks and re-enable the
hardware acceleration emulation.

Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Don Koch <dkoch@verizon.com>
---
 hw/display/vmware_vga.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 0c36c72..ec63290 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -29,8 +29,10 @@
 #include "hw/pci/pci.h"
 
 #undef VERBOSE
+#if 0
 #define HW_RECT_ACCEL
 #define HW_FILL_ACCEL
+#endif
 #define HW_MOUSE_ACCEL
 
 #include "vga_int.h"
-- 
1.8.3.1

[Qemu-devel] [PULL 2/5] vmware-vga: add vmsvga_verify_rect

[Gerd Hoffmann]

Add verification function for rectangles, returning
true if verification passes and false otherwise.

Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Don Koch <dkoch@verizon.com>
---
 hw/display/vmware_vga.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index ec63290..ba73a1c 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -294,8 +294,59 @@ enum {
     SVGA_CURSOR_ON_RESTORE_TO_FB = 3,
 };
 
+static inline bool vmsvga_verify_rect(DisplaySurface *surface,
+                                      const char *name,
+                                      int x, int y, int w, int h)
+{
+    if (x < 0) {
+        fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
+        return false;
+    }
+    if (x > SVGA_MAX_WIDTH) {
+        fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
+        return false;
+    }
+    if (w < 0) {
+        fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
+        return false;
+    }
+    if (w > SVGA_MAX_WIDTH) {
+        fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
+        return false;
+    }
+    if (x + w > surface_width(surface)) {
+        fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
+                name, surface_width(surface), x, w);
+        return false;
+    }
+
+    if (y < 0) {
+        fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
+        return false;
+    }
+    if (y > SVGA_MAX_HEIGHT) {
+        fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
+        return false;
+    }
+    if (h < 0) {
+        fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
+        return false;
+    }
+    if (h > SVGA_MAX_HEIGHT) {
+        fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
+        return false;
+    }
+    if (y + h > surface_height(surface)) {
+        fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
+                name, surface_height(surface), y, h);
+        return false;
+    }
+
+    return true;
+}
+
 static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
-                int x, int y, int w, int h)
+                                      int x, int y, int w, int h)
 {
     DisplaySurface *surface = qemu_console_surface(s->vga.con);
     int line;
-- 
1.8.3.1

[Qemu-devel] [PULL 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect

[Gerd Hoffmann]

Switch vmsvga_update_rect over to use vmsvga_verify_rect.  Slight change
in behavior:  We don't try to automatically fixup rectangles any more.
In case we find invalid update requests we'll do a full-screen update
instead.

Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Don Koch <dkoch@verizon.com>
---
 hw/display/vmware_vga.c | 32 ++++----------------------------
 1 file changed, 4 insertions(+), 28 deletions(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index ba73a1c..9d79de6 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -356,36 +356,12 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
     uint8_t *src;
     uint8_t *dst;
 
-    if (x < 0) {
-        fprintf(stderr, "%s: update x was < 0 (%d)\n", __func__, x);
-        w += x;
+    if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) {
+        /* go for a fullscreen update as fallback */
         x = 0;
-    }
-    if (w < 0) {
-        fprintf(stderr, "%s: update w was < 0 (%d)\n", __func__, w);
-        w = 0;
-    }
-    if (x + w > surface_width(surface)) {
-        fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
-                __func__, x, w);
-        x = MIN(x, surface_width(surface));
-        w = surface_width(surface) - x;
-    }
-
-    if (y < 0) {
-        fprintf(stderr, "%s: update y was < 0 (%d)\n",  __func__, y);
-        h += y;
         y = 0;
-    }
-    if (h < 0) {
-        fprintf(stderr, "%s: update h was < 0 (%d)\n",  __func__, h);
-        h = 0;
-    }
-    if (y + h > surface_height(surface)) {
-        fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
-                __func__, y, h);
-        y = MIN(y, surface_height(surface));
-        h = surface_height(surface) - y;
+        w = surface_width(surface);
+        h = surface_height(surface);
     }
 
     bypl = surface_stride(surface);
-- 
1.8.3.1

[Qemu-devel] [PULL 4/5] vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect

[Gerd Hoffmann]

Add verification to vmsvga_copy_rect, re-enable HW_RECT_ACCEL.

Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Don Koch <dkoch@verizon.com>
---
 hw/display/vmware_vga.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 9d79de6..73a1b52 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -29,8 +29,8 @@
 #include "hw/pci/pci.h"
 
 #undef VERBOSE
-#if 0
 #define HW_RECT_ACCEL
+#if 0
 #define HW_FILL_ACCEL
 #endif
 #define HW_MOUSE_ACCEL
@@ -406,7 +406,7 @@ static inline void vmsvga_update_rect_flush(struct vmsvga_state_s *s)
 }
 
 #ifdef HW_RECT_ACCEL
-static inline void vmsvga_copy_rect(struct vmsvga_state_s *s,
+static inline int vmsvga_copy_rect(struct vmsvga_state_s *s,
                 int x0, int y0, int x1, int y1, int w, int h)
 {
     DisplaySurface *surface = qemu_console_surface(s->vga.con);
@@ -417,6 +417,13 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s *s,
     int line = h;
     uint8_t *ptr[2];
 
+    if (!vmsvga_verify_rect(surface, "vmsvga_copy_rect/src", x0, y0, w, h)) {
+        return -1;
+    }
+    if (!vmsvga_verify_rect(surface, "vmsvga_copy_rect/dst", x1, y1, w, h)) {
+        return -1;
+    }
+
     if (y1 > y0) {
         ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1);
         ptr[1] = vram + bypp * x1 + bypl * (y1 + h - 1);
@@ -432,6 +439,7 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s *s,
     }
 
     vmsvga_update_rect_delayed(s, x1, y1, w, h);
+    return 0;
 }
 #endif
 
@@ -625,12 +633,12 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             width = vmsvga_fifo_read(s);
             height = vmsvga_fifo_read(s);
 #ifdef HW_RECT_ACCEL
-            vmsvga_copy_rect(s, x, y, dx, dy, width, height);
-            break;
-#else
+            if (vmsvga_copy_rect(s, x, y, dx, dy, width, height) == 0) {
+                break;
+            }
+#endif
             args = 0;
             goto badcmd;
-#endif
 
         case SVGA_CMD_DEFINE_CURSOR:
             len -= 8;
-- 
1.8.3.1

[Qemu-devel] [PULL 5/5] vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect

[Gerd Hoffmann]

Add verification to vmsvga_fill_rect, re-enable HW_FILL_ACCEL.

Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Don Koch <dkoch@verizon.com>
---
 hw/display/vmware_vga.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 73a1b52..1751f19 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -30,9 +30,7 @@
 
 #undef VERBOSE
 #define HW_RECT_ACCEL
-#if 0
 #define HW_FILL_ACCEL
-#endif
 #define HW_MOUSE_ACCEL
 
 #include "vga_int.h"
@@ -444,7 +442,7 @@ static inline int vmsvga_copy_rect(struct vmsvga_state_s *s,
 #endif
 
 #ifdef HW_FILL_ACCEL
-static inline void vmsvga_fill_rect(struct vmsvga_state_s *s,
+static inline int vmsvga_fill_rect(struct vmsvga_state_s *s,
                 uint32_t c, int x, int y, int w, int h)
 {
     DisplaySurface *surface = qemu_console_surface(s->vga.con);
@@ -457,6 +455,10 @@ static inline void vmsvga_fill_rect(struct vmsvga_state_s *s,
     uint8_t *src;
     uint8_t col[4];
 
+    if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) {
+        return -1;
+    }
+
     col[0] = c;
     col[1] = c >> 8;
     col[2] = c >> 16;
@@ -481,6 +483,7 @@ static inline void vmsvga_fill_rect(struct vmsvga_state_s *s,
     }
 
     vmsvga_update_rect_delayed(s, x, y, w, h);
+    return 0;
 }
 #endif
 
@@ -613,12 +616,12 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             width = vmsvga_fifo_read(s);
             height = vmsvga_fifo_read(s);
 #ifdef HW_FILL_ACCEL
-            vmsvga_fill_rect(s, colour, x, y, width, height);
-            break;
-#else
+            if (vmsvga_fill_rect(s, colour, x, y, width, height) == 0) {
+                break;
+            }
+#endif
             args = 0;
             goto badcmd;
-#endif
 
         case SVGA_CMD_RECT_COPY:
             len -= 7;
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 0/5] vmware-vga: add rectangle verification (CVE-2014-3689)

[Peter Maydell]

On 29 October 2014 11:56, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> After three revisions now finally the
> pull req for the CVE-2014-3689 fixes.
>
> please pull,
>   Gerd
>
> The following changes since commit 3e9418e160cd8901c83a3c88967158084f5b5c03:
>
>   Revert "main-loop.c: Handle SIGINT, SIGHUP and SIGTERM synchronously" (2014-10-27 15:05:09 +0000)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/pull-cve-2014-3689-20141029-1
>
> for you to fetch changes up to bd9ccd8517e83b7c33a9167815dbfffb30d70b13:
>
>   vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect (2014-10-29 12:01:30 +0100)
>
> ----------------------------------------------------------------
> vmware-vga: add rectangle verification (CVE-2014-3689)
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM