From 5418e8007c248bf9668d22a8c1fa9528149b69f2 Mon Sep 17 00:00:00 2001 From: Josef Gajdusek Date: Mon, 14 Nov 2016 11:39:01 +0100 Subject: Fix heap overflows in the various rectangle fill functions Altough rfbproto.c does check whether the overall FramebufferUpdate rectangle is too large, some of the individual encoding decoders do not, which allows a malicious server to overwrite parts of the heap. --- libvncclient/rfbproto.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'libvncclient') diff --git a/libvncclient/rfbproto.c b/libvncclient/rfbproto.c index 94b9bdb..9edfbad 100644 --- a/libvncclient/rfbproto.c +++ b/libvncclient/rfbproto.c @@ -147,6 +147,10 @@ void* rfbClientGetClientData(rfbClient* client, void* tag) /* messages */ +static boolean CheckRect(rfbClient* client, int x, int y, int w, int h) { + return x + w <= client->width && y + h <= client->height; +} + static void FillRectangle(rfbClient* client, int x, int y, int w, int h, uint32_t colour) { int i,j; @@ -154,6 +158,11 @@ static void FillRectangle(rfbClient* client, int x, int y, int w, int h, uint32_ return; } + if (!CheckRect(client, x, y, w, h)) { + rfbClientLog("Rect out of bounds: %dx%d at (%d, %d)\n", x, y, w, h); + return; + } + #define FILL_RECT(BPP) \ for(j=y*client->width;j<(y+h)*client->width;j+=client->width) \ for(i=x;iwidth * BPP / 8; \ @@ -201,6 +215,16 @@ static void CopyRectangleFromRectangle(rfbClient* client, int src_x, int src_y, return; } + if (!CheckRect(client, src_x, src_y, w, h)) { + rfbClientLog("Source rect out of bounds: %dx%d at (%d, %d)\n", src_x, src_y, w, h); + return; + } + + if (!CheckRect(client, dest_x, dest_y, w, h)) { + rfbClientLog("Dest rect out of bounds: %dx%d at (%d, %d)\n", dest_x, dest_y, w, h); + return; + } + #define COPY_RECT_FROM_RECT(BPP) \ { \ uint##BPP##_t* _buffer=((uint##BPP##_t*)client->frameBuffer)+(src_y-dest_y)*client->width+src_x-dest_x; \ -- cgit v1.2.3 From 5fff4353f66427b467eb29e5fdc1da4f2be028bb Mon Sep 17 00:00:00 2001 From: Josef Gajdusek Date: Mon, 14 Nov 2016 12:38:05 +0100 Subject: Fix heap overflow in the ultra.c decoder The Ultra type tile decoder does not use the _safe variant of the LZO decompress function, which allows a maliciuous server to overwrite parts of the heap by sending a larger-than-specified LZO data stream. --- libvncclient/ultra.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'libvncclient') diff --git a/libvncclient/ultra.c b/libvncclient/ultra.c index dac89b5..32a1b2b 100644 --- a/libvncclient/ultra.c +++ b/libvncclient/ultra.c @@ -86,14 +86,14 @@ HandleUltraBPP (rfbClient* client, int rx, int ry, int rw, int rh) /* uncompress the data */ uncompressedBytes = client->raw_buffer_size; - inflateResult = lzo1x_decompress( + inflateResult = lzo1x_decompress_safe( (lzo_byte *)client->ultra_buffer, toRead, (lzo_byte *)client->raw_buffer, (lzo_uintp) &uncompressedBytes, NULL); - + /* Note that uncompressedBytes will be 0 on output overrun */ if ((rw * rh * (BPP / 8)) != uncompressedBytes) - rfbClientLog("Ultra decompressed too little (%d < %d)", (rw * rh * (BPP / 8)), uncompressedBytes); + rfbClientLog("Ultra decompressed unexpected amount of data (%d != %d)\n", (rw * rh * (BPP / 8)), uncompressedBytes); /* Put the uncompressed contents of the update on the screen. */ if ( inflateResult == LZO_E_OK ) @@ -168,7 +168,7 @@ HandleUltraZipBPP (rfbClient* client, int rx, int ry, int rw, int rh) /* uncompress the data */ uncompressedBytes = client->raw_buffer_size; - inflateResult = lzo1x_decompress( + inflateResult = lzo1x_decompress_safe( (lzo_byte *)client->ultra_buffer, toRead, (lzo_byte *)client->raw_buffer, &uncompressedBytes, NULL); if ( inflateResult != LZO_E_OK ) -- cgit v1.2.3