From 0073e4f694d5a51bb72ff12a5e8364b6e752e094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= Date: Mon, 26 Feb 2018 13:48:00 +0100 Subject: [PATCH] Validate client cut text length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Client-provided unsigned 32-bit cut text length is passed to various functions that expects argument of a different type. E.g. "RFB 003.003\n\001\006\0\0\0\xff\xff\xff\xff" string sent to the RFB server leads to 4294967295 msg.cct.length value that in turn is interpreted as -1 by rfbReadExact() and thus uninitialized str buffer with potentially sensitive data is passed to subsequent functions. This patch fixes it by checking for a maximal value that still can be processed correctly. It also corrects accepting length value of zero (malloc(0) is interpreted on differnet systems differently). Whether a client can make the server allocate up to 2 GB and cause a denial of service on memory-tight systems is kept without answer. A possible solution would be adding an arbitrary memory limit that is deemed safe. CVE-2018-7225 Signed-off-by: Petr Písař --- libvncserver/rfbserver.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/libvncserver/rfbserver.c b/libvncserver/rfbserver.c index 116c488..a9561fc 100644 --- a/libvncserver/rfbserver.c +++ b/libvncserver/rfbserver.c @@ -88,6 +88,12 @@ #include /* strftime() */ #include +/* SIZE_MAX */ +#include +/* PRIu32 */ +#include +/* INT_MAX */ +#include #ifdef LIBVNCSERVER_WITH_WEBSOCKETS #include "rfbssl.h" @@ -2575,7 +2581,21 @@ rfbProcessClientNormalMessage(rfbClientPtr 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 check that the value fits into all of them to + * prevent from misinterpretation and thus from accessing uninitialized + * memory. CVE-2018-7225 */ + if (msg.cct.length > SIZE_MAX || msg.cct.length > INT_MAX - sz_rfbClientCutTextMsg) { + rfbLog("rfbClientCutText: too big cut text length requested: %" PRIu32 "\n", + msg.cct.length); + rfbCloseClient(cl); + return; + } + + /* Allow zero-length client cut text. */ + str = (char *)malloc(msg.cct.length ? msg.cct.length : 1); if (str == NULL) { rfbLogPerror("rfbProcessClientNormalMessage: not enough memory"); rfbCloseClient(cl); -- 2.13.6