From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51181) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUdkd-0007YU-SP for qemu-devel@nongnu.org; Wed, 26 Aug 2015 12:40:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUdkZ-0008Ih-G3 for qemu-devel@nongnu.org; Wed, 26 Aug 2015 12:40:39 -0400 From: Gerd Hoffmann Date: Wed, 26 Aug 2015 18:40:22 +0200 Message-Id: <1440607222-18716-2-git-send-email-kraxel@redhat.com> In-Reply-To: <1440607222-18716-1-git-send-email-kraxel@redhat.com> References: <1440607222-18716-1-git-send-email-kraxel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PULL 1/1] vnc: fix memory corruption (CVE-2015-5225) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: qemu-stable@nongnu.org, Gerd Hoffmann The _cmp_bytes variable added by commit "bea60dd ui/vnc: fix potential memory corruption issues" can become negative. Result is (possibly exploitable) memory corruption. Reason for that is it uses the stride instead of bytes per scanline to apply limits. For the server surface is is actually fine. vnc creates that itself, there is never any padding and thus scanline length always equals stride. For the guest surface scanline length and stride are typically identical too, but it doesn't has to be that way. So add and use a new variable (guest_ll) for the guest scanline length. Also rename min_stride to line_bytes to make more clear what it actually is. Finally sprinkle in an assert() to make sure we never use a negative _cmp_bytes again. Reported-by: =E8=8C=83=E7=A5=9A=E8=87=B3(=E5=BA=93=E7=89=B9) Reviewed-by: P J P Signed-off-by: Gerd Hoffmann --- ui/vnc.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index e26973a..caf82f5 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2872,7 +2872,7 @@ static int vnc_refresh_server_surface(VncDisplay *v= d) pixman_image_get_width(vd->server)); int height =3D MIN(pixman_image_get_height(vd->guest.fb), pixman_image_get_height(vd->server)); - int cmp_bytes, server_stride, min_stride, guest_stride, y =3D 0; + int cmp_bytes, server_stride, line_bytes, guest_ll, guest_stride, y = =3D 0; uint8_t *guest_row0 =3D NULL, *server_row0; VncState *vs; int has_dirty =3D 0; @@ -2891,17 +2891,21 @@ static int vnc_refresh_server_surface(VncDisplay = *vd) * Update server dirty map. */ server_row0 =3D (uint8_t *)pixman_image_get_data(vd->server); - server_stride =3D guest_stride =3D pixman_image_get_stride(vd->serve= r); + server_stride =3D guest_stride =3D guest_ll =3D + pixman_image_get_stride(vd->server); cmp_bytes =3D MIN(VNC_DIRTY_PIXELS_PER_BIT * VNC_SERVER_FB_BYTES, server_stride); if (vd->guest.format !=3D VNC_SERVER_FB_FORMAT) { int width =3D pixman_image_get_width(vd->server); tmpbuf =3D qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, widt= h); } else { + int guest_bpp =3D + PIXMAN_FORMAT_BPP(pixman_image_get_format(vd->guest.fb)); guest_row0 =3D (uint8_t *)pixman_image_get_data(vd->guest.fb); guest_stride =3D pixman_image_get_stride(vd->guest.fb); + guest_ll =3D pixman_image_get_width(vd->guest.fb) * ((guest_bpp = + 7) / 8); } - min_stride =3D MIN(server_stride, guest_stride); + line_bytes =3D MIN(server_stride, guest_ll); =20 for (;;) { int x; @@ -2932,9 +2936,10 @@ static int vnc_refresh_server_surface(VncDisplay *= vd) if (!test_and_clear_bit(x, vd->guest.dirty[y])) { continue; } - if ((x + 1) * cmp_bytes > min_stride) { - _cmp_bytes =3D min_stride - x * cmp_bytes; + if ((x + 1) * cmp_bytes > line_bytes) { + _cmp_bytes =3D line_bytes - x * cmp_bytes; } + assert(_cmp_bytes >=3D 0); if (memcmp(server_ptr, guest_ptr, _cmp_bytes) =3D=3D 0) { continue; } --=20 1.8.3.1