[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#770501: marked as done (unblock: libvncserver/0.9.9+dfsg-6.1)



Your message dated Fri, 28 Nov 2014 23:37:23 +0100
with message-id <5478F923.3020002@thykier.net>
and subject line Re: Bug#770501: unblock: libvncserver/0.9.9+dfsg-6.1
has caused the Debian Bug report #770501,
regarding unblock: libvncserver/0.9.9+dfsg-6.1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
770501: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=770501
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@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 <dktrkranz@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 <nruff@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 <github@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

--- End Message ---
--- Begin Message ---
On 2014-11-28 21:41, Tobias Frost wrote:
> Dear release team,
> 
> my NMU fixing #7626745 is now in the archive iIt fixes 4 CVE's
> I avoided the (formal) API break Luca mentioned, even if the API should
> not be used in the wild (its an exported local function which is never
> declared by any header file) 
> 
> Thanks for unblocking! 
> 
> --
> tobi
> 
> [...]

Unblocked, thanks.

~Niels

--- End Message ---

Reply to: