Petr Písař
7 years ago
parent
1d603de0c5
commit
46d9c246b9
@ -0,0 +1,40 @@
|
|||||||
|
From e7d578afbb16592ccee8f13aedd65b2220e220ae Mon Sep 17 00:00:00 2001
|
||||||
|
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
|
||||||
|
Date: Tue, 6 Mar 2018 11:58:02 +0100
|
||||||
|
Subject: [PATCH] Limit client cut text length to 1 MB
|
||||||
|
MIME-Version: 1.0
|
||||||
|
Content-Type: text/plain; charset=UTF-8
|
||||||
|
Content-Transfer-Encoding: 8bit
|
||||||
|
|
||||||
|
This patch constrains client 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 denial of service.
|
||||||
|
|
||||||
|
I keep the previous checks for maximal type values intentionally as
|
||||||
|
a course of defensive coding. (You cannot never know how small the
|
||||||
|
types are. And as a warning for people patching out this change not to
|
||||||
|
introduce CVE-2018-7225 again.)
|
||||||
|
|
||||||
|
Signed-off-by: Petr Písař <ppisar@redhat.com>
|
||||||
|
---
|
||||||
|
libvncserver/rfbserver.c | 4 +++-
|
||||||
|
1 file changed, 3 insertions(+), 1 deletion(-)
|
||||||
|
|
||||||
|
diff --git a/libvncserver/rfbserver.c b/libvncserver/rfbserver.c
|
||||||
|
index a9561fc..0027343 100644
|
||||||
|
--- a/libvncserver/rfbserver.c
|
||||||
|
+++ b/libvncserver/rfbserver.c
|
||||||
|
@@ -2587,7 +2587,9 @@ rfbProcessClientNormalMessage(rfbClientPtr cl)
|
||||||
|
* 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) {
|
||||||
|
+ /* But first to prevent from a denial-of-service by allocating to much
|
||||||
|
+ * memory in the server, we impose a limit of 1 MB. */
|
||||||
|
+ if (msg.cct.length > 1<<20 || 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);
|
||||||
|
--
|
||||||
|
2.13.6
|
||||||
|
|
||||||
@ -0,0 +1,76 @@
|
|||||||
|
From 0073e4f694d5a51bb72ff12a5e8364b6e752e094 Mon Sep 17 00:00:00 2001
|
||||||
|
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
|
||||||
|
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
|
||||||
|
<https://github.com/LibVNC/libvncserver/issues/218>
|
||||||
|
|
||||||
|
Signed-off-by: Petr Písař <ppisar@redhat.com>
|
||||||
|
---
|
||||||
|
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 <errno.h>
|
||||||
|
/* strftime() */
|
||||||
|
#include <time.h>
|
||||||
|
+/* SIZE_MAX */
|
||||||
|
+#include <stdint.h>
|
||||||
|
+/* PRIu32 */
|
||||||
|
+#include <inttypes.h>
|
||||||
|
+/* INT_MAX */
|
||||||
|
+#include <limits.h>
|
||||||
|
|
||||||
|
#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
|
||||||
|
|
||||||
Loading…
Reference in new issue