A malicious VNC client can send a SetPixelFormat message with shift values >= 32, causing UB mask computation (e.g. red_max << red_shift where red_shift is 255). Apparently, this is not covered by -fwrapv. Reject color shifts >= bits_per_pixel before computing masks, and cast the max values to uint32_t to avoid signed integer overflow when a valid shift (e.g. 24) would set bit 31 of a signed int. Fixes: 9f64916da20 ("pixman/vnc: use pixman images in vnc.") Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3948 Reported-by: huntr bubble Signed-off-by: Marc-Andre Lureau --- ui/vnc.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 559d3954b87..94a38242c56 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2276,18 +2276,25 @@ static void set_pixel_format(VncState *vs, int bits_per_pixel, return; } + if (red_shift >= bits_per_pixel || + green_shift >= bits_per_pixel || + blue_shift >= bits_per_pixel) { + vnc_client_error(vs); + return; + } + vs->client_pf.rmax = red_max ? red_max : 0xFF; vs->client_pf.rbits = ctpopl(red_max); vs->client_pf.rshift = red_shift; - vs->client_pf.rmask = red_max << red_shift; + vs->client_pf.rmask = (uint32_t)red_max << red_shift; vs->client_pf.gmax = green_max ? green_max : 0xFF; vs->client_pf.gbits = ctpopl(green_max); vs->client_pf.gshift = green_shift; - vs->client_pf.gmask = green_max << green_shift; + vs->client_pf.gmask = (uint32_t)green_max << green_shift; vs->client_pf.bmax = blue_max ? blue_max : 0xFF; vs->client_pf.bbits = ctpopl(blue_max); vs->client_pf.bshift = blue_shift; - vs->client_pf.bmask = blue_max << blue_shift; + vs->client_pf.bmask = (uint32_t)blue_max << blue_shift; vs->client_pf.bits_per_pixel = bits_per_pixel; vs->client_pf.bytes_per_pixel = bits_per_pixel / 8; vs->client_pf.depth = bits_per_pixel == 32 ? 24 : bits_per_pixel; -- 2.55.0