From 045a044e8ae79db9244593fbce154cdf6e843273 Mon Sep 17 00:00:00 2001 From: newsoft Date: Fri, 15 Aug 2014 16:31:13 +0200 Subject: Fix integer overflow in MallocFrameBuffer() Promote integers to uint64_t to avoid integer overflow issue during frame buffer allocation for very large screen sizes --- libvncclient/vncviewer.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/libvncclient/vncviewer.c b/libvncclient/vncviewer.c index 3b16a6f..24bc6f8 100644 --- a/libvncclient/vncviewer.c +++ b/libvncclient/vncviewer.c @@ -82,9 +82,27 @@ static char* ReadPassword(rfbClient* client) { #endif } static rfbBool MallocFrameBuffer(rfbClient* client) { +uint64_t allocSize; + if(client->frameBuffer) free(client->frameBuffer); - client->frameBuffer=malloc(client->width*client->height*client->format.bitsPerPixel/8); + + /* SECURITY: promote 'width' into uint64_t so that the multiplication does not overflow + 'width' and 'height' are 16-bit integers per RFB protocol design + SIZE_MAX is the maximum value that can fit into size_t + */ + allocSize = (uint64_t)client->width * client->height * client->format.bitsPerPixel/8; + + if (allocSize >= SIZE_MAX) { + rfbClientErr("CRITICAL: cannot allocate frameBuffer, requested size is too large\n"); + return FALSE; + } + + client->frameBuffer=malloc( (size_t)allocSize ); + + if (client->frameBuffer == NULL) + rfbClientErr("CRITICAL: frameBuffer allocation failed, requested size too large or not enough memory?\n"); + return client->frameBuffer?TRUE:FALSE; } -- cgit v1.1 From 85a778c0e45e87e35ee7199f1f25020648e8b812 Mon Sep 17 00:00:00 2001 From: newsoft Date: Fri, 15 Aug 2014 16:41:58 +0200 Subject: Check for MallocFrameBuffer() return value If MallocFrameBuffer() returns FALSE, frame buffer pointer is left to NULL. Subsequent writes into that buffer could lead to memory corruption, or even arbitrary code execution. --- libvncclient/rfbproto.c | 10 +++++++--- libvncclient/vncviewer.c | 3 ++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/libvncclient/rfbproto.c b/libvncclient/rfbproto.c index b4d7156..f55c74f 100644 --- a/libvncclient/rfbproto.c +++ b/libvncclient/rfbproto.c @@ -1829,7 +1829,8 @@ HandleRFBServerMessage(rfbClient* client) client->updateRect.x = client->updateRect.y = 0; client->updateRect.w = client->width; client->updateRect.h = client->height; - client->MallocFrameBuffer(client); + if (!client->MallocFrameBuffer(client)) + return FALSE; SendFramebufferUpdateRequest(client, 0, 0, rect.r.w, rect.r.h, FALSE); rfbClientLog("Got new framebuffer size: %dx%d\n", rect.r.w, rect.r.h); continue; @@ -2290,7 +2291,9 @@ HandleRFBServerMessage(rfbClient* client) client->updateRect.x = client->updateRect.y = 0; client->updateRect.w = client->width; client->updateRect.h = client->height; - client->MallocFrameBuffer(client); + if (!client->MallocFrameBuffer(client)) + return FALSE; + SendFramebufferUpdateRequest(client, 0, 0, client->width, client->height, FALSE); rfbClientLog("Got new framebuffer size: %dx%d\n", client->width, client->height); break; @@ -2306,7 +2309,8 @@ HandleRFBServerMessage(rfbClient* client) client->updateRect.x = client->updateRect.y = 0; client->updateRect.w = client->width; client->updateRect.h = client->height; - client->MallocFrameBuffer(client); + if (!client->MallocFrameBuffer(client)) + return FALSE; SendFramebufferUpdateRequest(client, 0, 0, client->width, client->height, FALSE); rfbClientLog("Got new framebuffer size: %dx%d\n", client->width, client->height); break; diff --git a/libvncclient/vncviewer.c b/libvncclient/vncviewer.c index 24bc6f8..65b7412 100644 --- a/libvncclient/vncviewer.c +++ b/libvncclient/vncviewer.c @@ -250,7 +250,8 @@ static rfbBool rfbInitConnection(rfbClient* client) client->width=client->si.framebufferWidth; client->height=client->si.framebufferHeight; - client->MallocFrameBuffer(client); + if (!client->MallocFrameBuffer(client)) + return FALSE; if (!SetFormatAndEncodings(client)) return FALSE; -- cgit v1.1