Patchwork Re: [PATCH to consider for 0.12] vmware_vga: Don't crash on too-big DEFINE_CURSOR command

login
register
mail settings
Submitter Roland Dreier
Date Dec. 20, 2009, 6:06 p.m.
Message ID <aday6kx1pwx.fsf@roland-alpha.cisco.com>
Download mbox | patch
Permalink /patch/41512/
State New
Headers show

Comments

Roland Dreier - Dec. 20, 2009, 6:06 p.m.
I see that you applied Dave's series, no worries on that, but I think we
really want the following, since a clever enough malicious guest can
probably turn the overrun into code execution in the host QEMU (though
I've not tried writing an exploit or anything like that)... not sure if
this merits the whole CVE process but it does look very much worth
fixing, and probably the other guest commands want auditing too...

Patch

===
vmware_vga: Check cursor dimensions passed from guest to avoid buffer overflow

Check that the cursor dimensions passed from the guest for the
DEFINE_CURSOR command don't overflow the available space in the
cursor.image[] or cursor.mask[] arrays before copying data from the
guest into those arrays.

Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
 hw/vmware_vga.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 7ab1c79..5e969ae 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -562,6 +562,13 @@  static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             cursor.height = y = vmsvga_fifo_read(s);
             vmsvga_fifo_read(s);
             cursor.bpp = vmsvga_fifo_read(s);
+
+	    if (SVGA_BITMAP_SIZE(x, y) > sizeof cursor.mask ||
+		SVGA_PIXMAP_SIZE(x, y, cursor.bpp) > sizeof cursor.image) {
+		    args = SVGA_BITMAP_SIZE(x, y) + SVGA_PIXMAP_SIZE(x, y, cursor.bpp);
+		    goto badcmd;
+	    }
+
             for (args = 0; args < SVGA_BITMAP_SIZE(x, y); args ++)
                 cursor.mask[args] = vmsvga_fifo_read_raw(s);
             for (args = 0; args < SVGA_PIXMAP_SIZE(x, y, cursor.bpp); args ++)