From 97815b3be1cf385e65d525b9075d2e1a31f57228 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Wed, 26 Nov 2008 14:14:06 +0000 Subject: [PATCH] vnc and xenfb integer overflow and division by zero vuln fixes 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 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 Modified-by: Ian Jackson Signed-off-by: Ian Jackson 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 --- hw/xenfb.c | 2 +- vnc.c | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/hw/xenfb.c b/hw/xenfb.c index 41ede97c..f235658b 100644 --- a/hw/xenfb.c +++ b/hw/xenfb.c @@ -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 2cf3a31a..01e22e54 100644 --- 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) -- 2.39.5