Limit client cut text length to 1 MB

This patch constrains a client cut text length to 1 MB. Otherwise
a client could make server allocate 2 GB of memory and that seems to
be to much to classify it as a denial of service.

The limit also prevents from an integer overflow followed by copying
an uninitilized memory when processing msg.cct.length value larger
than SIZE_MAX or INT_MAX - sz_rfbClientCutTextMsg.

This patch also corrects accepting length value of zero (malloc(0) is
interpreted on differnet systems differently).

CVE-2018-7225
<https://github.com/LibVNC/libvncserver/issues/218>
(cherry picked from commit 28afb6c537dc82ba04d5f245b15ca7205c6dbb9c)

1 files changed, 20 insertions, 1 deletions

diff --git a/krfb/libvncserver/rfbserver.c b/krfb/libvncserver/rfbserver.c
index 66cc4ee3..5725d068 100644
--- a/krfb/libvncserver/rfbserver.c
+++ b/krfb/libvncserver/rfbserver.c
@@ -52,6 +52,9 @@ typedef int socklen_t;
 #define DEBUGPROTO(x)
 #endif
 
+/* PRIu32 */
+#include <inttypes.h>
+
 rfbClientPtr pointerClient = NULL;  /* Mutex for pointer events */
 
 static void rfbProcessClientProtocolVersion(rfbClientPtr cl);
@@ -898,7 +901,23 @@ rfbProcessClientNormalMessage(cl)
 
         msg.cct.length = Swap32IfLE(msg.cct.length);
 
-        str = (char *)malloc(msg.cct.length);
+	/* uint32_t input is passed to malloc()'s size_t argument,
+	 * to rfbReadExact()'s int argument, to rfbStatRecordMessageRcvd()'s int
+	 * argument increased of sz_rfbClientCutTextMsg, and to setXCutText()'s int
+	 * argument. Here we impose a limit of 1 MB so that the value fits
+	 * into all of the types to prevent from misinterpretation and thus
+	 * from accessing uninitialized memory (CVE-2018-7225) and also to
+	 * prevent from a denial-of-service by allocating to much memory in
+	 * the server. */
+	if (msg.cct.length > 1<<20) {
+	    rfbLog("rfbClientCutText: too big cut text length requested: %" PRIu32 "\n",
+		    msg.cct.length);
+	    rfbCloseClient(cl);
+	    return;
+	}
+
+	/* Allow zero-length client cut text. */
+	str = (char *)calloc(msg.cct.length ? msg.cct.length : 1, 1);
 
         if ((n = ReadExact(cl, str, msg.cct.length)) <= 0) {
             if (n != 0)