Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: unblock
I prepared an upload to fix grave bug #762745, cherry-picking patches from
upstream to fix multiple vulnerabilities. I also took the chance to include
fixes for #766257 and #758754. Please tell me whether I should drop these
instead.
The problem with this (tentative) upload is related to this commit, needed to
fix CVE-2014-6055:
https://github.com/newsoft/libvncserver/commit/06ccdf016154fde8eccb5355613ba04c59127b2e
It seems there's an API/ABI break, also reported in RedHat, as per
https://bugzilla.redhat.com/show_bug.cgi?id=1144293#c2, which could be
problematic at this point. What would be the recommended way to proceed from
here?
-- System Information:
Debian Release: jessie/sid
APT prefers testing-updates
APT policy: (500, 'testing-updates'), (500, 'testing')
Architecture: amd64 (x86_64)
Kernel: Linux 3.16-3-amd64 (SMP w/8 CPU cores)
diff --git a/debian/changelog b/debian/changelog
index 4cb521d..77de074 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,19 @@
+libvncserver (0.9.9+dfsg-7) UNRELEASED; urgency=medium
+
+ * debian/patches/CVE-2014.patch:
+ - Fix multiple vulnerabilities (Closes: #762745).
+ - CVE-2014-6051 Integer overflow in MallocFrameBuffer() on client side.
+ - CVE-2014-6052 Lack of malloc() return value checking on client side.
+ - CVE-2014-6053 Server crash on a very large ClientCutText message.
+ - CVE-2014-6054 Server crash when scaling factor is set to zero.
+ - CVE-2014-6055 Multiple stack overflows in File Transfer feature.
+ * debian/patches/novnc.patch:
+ - Set opcode correctly for binary frames (Closes: #766257).
+ * debian/control:
+ - Use uppercase VNC in short descriptions (Closes: #758754).
+
+ -- Luca Falavigna <dktrkr...@debian.org> Fri, 21 Nov 2014 20:51:50 +0100
+
libvncserver (0.9.9+dfsg-6) unstable; urgency=medium
[ Luca Falavigna ]
diff --git a/debian/control b/debian/control
index 9e188be..17ebbe6 100644
--- a/debian/control
+++ b/debian/control
@@ -22,7 +22,7 @@ Depends: ${shlibs:Depends},
Breaks: libvncserver0 (<< 0.9.9+dfsg-3)
Replaces: libvncserver0 (<< 0.9.9+dfsg-3)
Multi-Arch: same
-Description: API to write one's own vnc server - client library
+Description: API to write one's own VNC server - client library
LibVNCServer makes writing a VNC server (or more correctly, a program
exporting a framebuffer via the Remote Frame Buffer protocol) easy. It hides
the programmer from the tedious task of managing clients and compression.
@@ -35,7 +35,7 @@ Pre-Depends: ${misc:Pre-Depends}
Depends: ${shlibs:Depends},
${misc:Depends}
Multi-Arch: same
-Description: API to write one's own vnc server
+Description: API to write one's own VNC server
LibVNCServer makes writing a VNC server (or more correctly, a program
exporting a framebuffer via the Remote Frame Buffer protocol) easy. It hides
the programmer from the tedious task of managing clients and compression.
@@ -53,7 +53,7 @@ Depends: ${misc:Depends},
zlib1g-dev,
libvncserver-config
Multi-Arch: same
-Description: API to write one's own vnc server - development files
+Description: API to write one's own VNC server - development files
LibVNCServer makes writing a VNC server (or more correctly, a program
exporting a framebuffer via the Remote Frame Buffer protocol) easy. It hides
the programmer from the tedious task of managing clients and compression.
@@ -68,7 +68,7 @@ Depends: ${misc:Depends},
libvncclient0 (= ${binary:Version}),
libvncserver0 (= ${binary:Version})
Multi-Arch: foreign
-Description: API to write one's own vnc server - library utility
+Description: API to write one's own VNC server - library utility
LibVNCServer makes writing a VNC server (or more correctly, a program
exporting a framebuffer via the Remote Frame Buffer protocol) easy. It hides
the programmer from the tedious task of managing clients and compression.
diff --git a/debian/patches/CVE-2014.patch b/debian/patches/CVE-2014.patch
new file mode 100644
index 0000000..3931854
--- /dev/null
+++ b/debian/patches/CVE-2014.patch
@@ -0,0 +1,293 @@
+Description: Fix CVE-2014-6051 CVE-2014-6052 CVE-2014-6053
+ CVE-2014-6054 CVE-2014-6055
+Author: Nicolas Ruff <nr...@google.com>
+
+Index: libvncserver/libvncclient/rfbproto.c
+===================================================================
+--- libvncserver.orig/libvncclient/rfbproto.c 2014-11-21 20:35:19.488146693 +0100
++++ libvncserver/libvncclient/rfbproto.c 2014-11-21 20:45:30.973828663 +0100
+@@ -1807,7 +1807,8 @@
+ 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;
+@@ -2276,7 +2277,9 @@
+ 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;
+Index: libvncserver/libvncclient/vncviewer.c
+===================================================================
+--- libvncserver.orig/libvncclient/vncviewer.c 2014-11-21 20:35:19.488146693 +0100
++++ libvncserver/libvncclient/vncviewer.c 2014-11-21 20:45:30.973828663 +0100
+@@ -82,9 +82,27 @@
+ #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;
+ }
+
+@@ -225,7 +243,8 @@
+
+ client->width=client->si.framebufferWidth;
+ client->height=client->si.framebufferHeight;
+- client->MallocFrameBuffer(client);
++ if (!client->MallocFrameBuffer(client))
++ return FALSE;
+
+ if (!SetFormatAndEncodings(client))
+ return FALSE;
+Index: libvncserver/libvncserver/rfbserver.c
+===================================================================
+--- libvncserver.orig/libvncserver/rfbserver.c 2014-11-21 20:35:19.488146693 +0100
++++ libvncserver/libvncserver/rfbserver.c 2014-11-21 20:45:59.494260310 +0100
+@@ -1237,21 +1237,35 @@
+ #define RFB_FILE_ATTRIBUTE_TEMPORARY 0x100
+ #define RFB_FILE_ATTRIBUTE_COMPRESSED 0x800
+
+-rfbBool rfbFilenameTranslate2UNIX(rfbClientPtr cl, char *path, char *unixPath)
++rfbBool rfbFilenameTranslate2UNIX(rfbClientPtr cl, /* in */ char *path, /* out */ char *unixPath, size_t unixPathMaxLen )
+ {
+ int x;
+ char *home=NULL;
+
+ FILEXFER_ALLOWED_OR_CLOSE_AND_RETURN("", cl, FALSE);
+
++ /*
++ * Do not use strncpy() - truncating the file name would probably have undesirable side effects
++ * Instead check if destination buffer is big enough
++ */
++
++ if (strlen(path) >= unixPathMaxLen)
++ return FALSE;
++
+ /* C: */
+ if (path[0]=='C' && path[1]==':')
++ {
+ strcpy(unixPath, &path[2]);
++ }
+ else
+ {
+ home = getenv("HOME");
+ if (home!=NULL)
+ {
++ /* Re-check buffer size */
++ if ((strlen(path) + strlen(home) + 1) >= unixPathMaxLen)
++ return FALSE;
++
+ strcpy(unixPath, home);
+ strcat(unixPath,"/");
+ strcat(unixPath, path);
+@@ -1289,7 +1303,8 @@
+ FILEXFER_ALLOWED_OR_CLOSE_AND_RETURN("", cl, FALSE);
+
+ /* Client thinks we are Winblows */
+- rfbFilenameTranslate2UNIX(cl, buffer, path);
++ if (!rfbFilenameTranslate2UNIX(cl, buffer, path, sizeof(path)))
++ return FALSE;
+
+ if (DB) rfbLog("rfbProcessFileTransfer() rfbDirContentRequest: rfbRDirContent: \"%s\"->\"%s\"\n",buffer, path);
+
+@@ -1566,7 +1581,9 @@
+ /* add some space to the end of the buffer as we will be adding a timespec to it */
+ if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE;
+ /* The client requests a File */
+- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
++ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
++ goto fail;
++
+ cl->fileTransfer.fd=open(filename1, O_RDONLY, 0744);
+
+ /*
+@@ -1660,16 +1677,17 @@
+ */
+ if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE;
+
+- /* Parse the FileTime */
++ /* Parse the FileTime
++ * TODO: FileTime is actually never used afterwards
++ */
+ p = strrchr(buffer, ',');
+ if (p!=NULL) {
+ *p = '\0';
+- strcpy(szFileTime, p+1);
++ strncpy(szFileTime, p+1, sizeof(szFileTime));
++ szFileTime[sizeof(szFileTime)-1] = '\x00'; /* ensure NULL terminating byte is present, even if copy overflowed */
+ } else
+ szFileTime[0]=0;
+
+-
+-
+ /* Need to read in sizeHtmp */
+ if ((n = rfbReadExact(cl, (char *)&sizeHtmp, 4)) <= 0) {
+ if (n != 0)
+@@ -1681,7 +1699,8 @@
+ }
+ sizeHtmp = Swap32IfLE(sizeHtmp);
+
+- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
++ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
++ goto fail;
+
+ /* If the file exists... We can send a rfbFileChecksums back to the client before we send an rfbFileAcceptHeader */
+ /* TODO: Delta Transfer */
+@@ -1810,7 +1829,9 @@
+ if ((buffer = rfbProcessFileTransferReadBuffer(cl, length))==NULL) return FALSE;
+ switch (contentParam) {
+ case rfbCDirCreate: /* Client requests the creation of a directory */
+- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
++ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
++ goto fail;
++
+ retval = mkdir(filename1, 0755);
+ if (DB) rfbLog("rfbProcessFileTransfer() rfbCommand: rfbCDirCreate(\"%s\"->\"%s\") %s\n", buffer, filename1, (retval==-1?"Failed":"Success"));
+ /*
+@@ -1819,7 +1840,9 @@
+ if (buffer!=NULL) free(buffer);
+ return retval;
+ case rfbCFileDelete: /* Client requests the deletion of a file */
+- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
++ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
++ goto fail;
++
+ if (stat(filename1,&statbuf)==0)
+ {
+ if (S_ISDIR(statbuf.st_mode))
+@@ -1837,8 +1860,12 @@
+ {
+ /* Split into 2 filenames ('*' is a seperator) */
+ *p = '\0';
+- rfbFilenameTranslate2UNIX(cl, buffer, filename1);
+- rfbFilenameTranslate2UNIX(cl, p+1, filename2);
++ if (!rfbFilenameTranslate2UNIX(cl, buffer, filename1, sizeof(filename1)))
++ goto fail;
++
++ if (!rfbFilenameTranslate2UNIX(cl, p+1, filename2, sizeof(filename2)))
++ goto fail;
++
+ retval = rename(filename1,filename2);
+ if (DB) rfbLog("rfbProcessFileTransfer() rfbCommand: rfbCFileRename(\"%s\"->\"%s\" -->> \"%s\"->\"%s\") %s\n", buffer, filename1, p+1, filename2, (retval==-1?"Failed":"Success"));
+ /*
+@@ -1858,6 +1885,10 @@
+ /* NOTE: don't forget to free(buffer) if you return early! */
+ if (buffer!=NULL) free(buffer);
+ return TRUE;
++
++fail:
++ if (buffer!=NULL) free(buffer);
++ return FALSE;
+ }
+
+ /*
+@@ -2457,6 +2488,11 @@
+ msg.cct.length = Swap32IfLE(msg.cct.length);
+
+ str = (char *)malloc(msg.cct.length);
++ if (str == NULL) {
++ rfbLogPerror("rfbProcessClientNormalMessage: not enough memory");
++ rfbCloseClient(cl);
++ return;
++ }
+
+ if ((n = rfbReadExact(cl, str, msg.cct.length)) <= 0) {
+ if (n != 0)
+@@ -2498,6 +2534,20 @@
+ rfbCloseClient(cl);
+ return;
+ }
++
++ if (msg.ssc.scale == 0) {
++ rfbLogPerror("rfbProcessClientNormalMessage: will not accept a scale factor of zero");
++ rfbCloseClient(cl);
++ return;
++ }
++
++
++ if (msg.ssc.scale == 0) {
++ rfbLogPerror("rfbProcessClientNormalMessage: will not accept a scale factor of zero");
++ rfbCloseClient(cl);
++ return;
++ }
++
+ rfbStatRecordMessageRcvd(cl, msg.type, sz_rfbSetScaleMsg, sz_rfbSetScaleMsg);
+ rfbLog("rfbSetScale(%d)\n", msg.ssc.scale);
+ rfbScalingSetup(cl,cl->screen->width/msg.ssc.scale, cl->screen->height/msg.ssc.scale);
+Index: libvncserver/libvncserver/scale.c
+===================================================================
+--- libvncserver.orig/libvncserver/scale.c 2014-11-21 20:35:19.492147116 +0100
++++ libvncserver/libvncserver/scale.c 2014-11-21 20:46:34.088646966 +0100
+@@ -66,6 +66,10 @@
+ (double) ((int) (x)) : (double) ((int) (x) + 1) )
+ #define FLOOR(x) ( (double) ((int) (x)) )
+
++static inline int pad4(int value) {
++ int remainder = value & 3;
++ return value + (remainder == 0 ? 0 : 4 - remainder);
++}
+
+ int ScaleX(rfbScreenInfoPtr from, rfbScreenInfoPtr to, int x)
+ {
+@@ -281,14 +285,29 @@
+ ptr = malloc(sizeof(rfbScreenInfo));
+ if (ptr!=NULL)
+ {
++ int allocSize;
++
+ /* copy *everything* (we don't use most of it, but just in case) */
+ memcpy(ptr, cl->screen, sizeof(rfbScreenInfo));
++
++ /* SECURITY: make sure that no integer overflow will occur afterwards.
++ * Note: this is defensive coding, as the check should have already been
++ * performed during initial, non-scaled screen setup.
++ */
++ allocSize = pad4(width * (ptr->bitsPerPixel/8)); /* per protocol, width<2**16 and bpp<256 */
++ if ((height == 0) || (allocSize >= (SIZE_MAX / height)))
++ {
++ free(ptr);
++ return NULL; /* malloc() will allocate an incorrect buffer size - early abort */
++ }
++
++ /* Resume copy everything */
+ ptr->width = width;
+ ptr->height = height;
+ ptr->paddedWidthInBytes = (ptr->bitsPerPixel/8)*ptr->width;
+
+ /* Need to by multiples of 4 for Sparc systems */
+- ptr->paddedWidthInBytes += (ptr->paddedWidthInBytes % 4);
++ ptr->paddedWidthInBytes = pad4(ptr->paddedWidthInBytes);
+
+ /* Reset the reference count to 0! */
+ ptr->scaledScreenRefCount = 0;
diff --git a/debian/patches/novnc.patch b/debian/patches/novnc.patch
new file mode 100644
index 0000000..c81b742
--- /dev/null
+++ b/debian/patches/novnc.patch
@@ -0,0 +1,15 @@
+Description: Set opcode correctly for binary frames
+Author: Joel Martin <git...@martintribe.org>
+
+Index: libvncserver/libvncserver/websockets.c
+===================================================================
+--- libvncserver.orig/libvncserver/websockets.c 2014-11-21 20:35:19.492147116 +0100
++++ libvncserver/libvncserver/websockets.c 2014-11-21 20:49:23.374940701 +0100
+@@ -783,6 +783,7 @@
+ /* calculate the resulting size */
+ blen = B64LEN(len);
+ } else {
++ opcode = WS_OPCODE_BINARY_FRAME;
+ blen = len;
+ }
+
diff --git a/debian/patches/series b/debian/patches/series
index 158e82e..48391d2 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,3 +5,5 @@ multiarch.patch
listenSock.patch
ppc64el.patch
pkgconfig.patch
+CVE-2014.patch
+novnc.patch
