]> xenbits.xensource.com Git - qemu-xen-3.3-testing.git/commitdiff
vnc and xenfb integer overflow and division by zero vuln fixes
authorIan Jackson <ian.jackson@eu.citrix.com>
Wed, 26 Nov 2008 14:14:06 +0000 (14:14 +0000)
committerIan Jackson <Ian.Jackson@eu.citrix.com>
Wed, 26 Nov 2008 14:14:06 +0000 (14:14 +0000)
 row_stride_div0.patch: a malicious frontend can send row_stride==0 and force
 qemu-dm to perform division by 0
 vnc_resize_doublecheck.patch: there is an unchecked multiplication when
 calculating framebuffer size. Cs 17630 sanitizes framebuffer dimensions
 passed by the frontend, so most probably no integer overflow can happen, but
 there should be a check for overflow close to the actual computation (to
 make code review easier and to cope with other codepaths in the future).

 (Patches submitted by Rafal Wojtczuk <rafal@invisiblethingslab.com>
 against xen-3.2 ioemu; adapted for xen-unstable by Ian Jackson and also
 edited to actually compile and do be correct.)

Contributed-by: Rafal Wojtczuk <rafal@invisiblethingslab.com>
Modified-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cherry picked from xen-unsstable a83c1174b942d0f0f0e05927eb5b69fe8489b7ab

PLUS

 vnc integer overflow check fix overzealous zero checking

 In a83c1174b942d0f0f0e05927eb5b69fe8489b7ab, we arranged to avoid
 integer overflow and calls to realloc(nonzero,0).  However
 vs->depth==0 is legitimate on entry to vnc_dpy_resize_shared.

 We need to move the check for vs->depth until after vnc_colourdepth.

Cherry picked from xen-unstable 81b31c9f37ac4e3584bdfe8e7b04bedcb8940b88

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
hw/xenfb.c
vnc.c

index 41ede97c5caa03dae209ed21b4980008848872a4..f235658bbca4acc79519e2203dfd0294f2a87713 100644 (file)
@@ -511,7 +511,7 @@ static int xenfb_configure_fb(struct xenfb *xenfb, size_t fb_len_lim,
                        depth);
                return -1;
        }
-       if (row_stride < 0 || row_stride > fb_len) {
+       if (row_stride <= 0 || row_stride > fb_len) {
                fprintf(stderr,
                        "FB: invalid frontend stride %d\n", row_stride);
                return -1;
diff --git a/vnc.c b/vnc.c
index 2cf3a31af545c52b1f064d985c1f5d1ceb258819..01e22e544ad2669131abc13c216c0a3efbc3b54f 100644 (file)
--- a/vnc.c
+++ b/vnc.c
@@ -366,6 +366,13 @@ static void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
     vnc_write_s32(vs, encoding);
 }
 
+static int mult_overflows(int x, int y)
+{
+    if (x<=0 || y<=0 || x>=INT_MAX/y)
+        return 1;
+    else return 0;
+}
+
 static void vnc_dpy_resize_shared(DisplayState *ds, int w, int h, int depth, int linesize, void *pixels)
 {
     static int allocated;
@@ -374,6 +381,13 @@ static void vnc_dpy_resize_shared(DisplayState *ds, int w, int h, int depth, int
     int o;
 
     vnc_colourdepth(ds, depth);
+    if (mult_overflows(w, h) || mult_overflows(w*h, vs->depth) ||
+        mult_overflows(h, sizeof(vs->dirty_row[0]))) {
+        fprintf(stderr, "vnc: suspicious vnc_dpy_resize arguments"
+               " (w=%d h=%d depth=%d linesize=%d vs->depth=%d), exiting\n",
+               w, h, depth, linesize, vs->depth);
+        exit(1);
+    }
     if (!ds->shared_buf) {
         ds->linesize = w * vs->depth;
         if (allocated)