rpms/tigervnc
6
0
Fork 0
Code Issues Pull Requests Releases Activity

Bug 1438704 - CVE-2017-7392 CVE-2017-7393 CVE-2017-7394

Browse Source 
CVE-2017-7395 CVE-2017-7396 tigervnc: various flaws
            + other upstream related fixes
This commit is contained in:
Jan Grulich 2017-04-04 12:52:23 +02:00
parent 71c6a70c05
commit 491ae3ae9c
9 changed files with 365 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
tigervnc-avoid-leaking-shared-memory.patch Normal file
View File

@ -0,0 +1,20 @@
From fbbd48b35e53fb156b91715dae4aab9008533565 Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Wed, 29 Mar 2017 13:27:32 +0200
Subject: Avoid leaking shared memory via X server
It's not enough that we detach from the shared memory, we must also
tell the X server to do so for it to be freed properly.
diff --git a/vncviewer/X11PixelBuffer.cxx b/vncviewer/X11PixelBuffer.cxx
index ce5c4d8..ac35dfc 100644
--- a/vncviewer/X11PixelBuffer.cxx
+++ b/vncviewer/X11PixelBuffer.cxx
@@ -126,6 +126,7 @@ X11PixelBuffer::~X11PixelBuffer()
{
if (shminfo) {
vlog.debug("Freeing shared memory XImage");
+ XShmDetach(fl_display, shminfo);
shmList.remove(this);
Fl::remove_system_handler(handleSystemEvent);
shmdt(shminfo->shmaddr);

20
tigervnc-be-more-restrictive-with-shared-memory-mode-bits.patch Normal file
View File

@ -0,0 +1,20 @@
From d71508b94bd1c6f0d8be89aa559a8a7de48f7f3f Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Wed, 29 Mar 2017 13:28:55 +0200
Subject: Be more restrictive with shared memory mode bits
Everyone else seems to get by with using 0600, so let's do the same.
diff --git a/vncviewer/X11PixelBuffer.cxx b/vncviewer/X11PixelBuffer.cxx
index ce5c4d8..a1673da 100644
--- a/vncviewer/X11PixelBuffer.cxx
+++ b/vncviewer/X11PixelBuffer.cxx
@@ -189,7 +190,7 @@ int X11PixelBuffer::setupShm()
shminfo->shmid = shmget(IPC_PRIVATE,
xim->bytes_per_line * xim->height,
- IPC_CREAT|0777);
+ IPC_CREAT|0600);
if (shminfo->shmid == -1)
goto free_xim;

20
tigervnc-delete-underlying-ssecurity.patch Normal file
View File

@ -0,0 +1,20 @@
From 8f3e8663b3cf57c0b62d939d6953fbfcc112aadd Mon Sep 17 00:00:00 2001
From: Michal Srb <michalsrb@gmail.com>
Date: Wed, 29 Mar 2017 16:23:18 +0300
Subject: Delete underlying ssecurity in SSecurityVeNCrypt.
Otherwise it gets leaked which would allow even not authenticated clients to exhaust server memory.
diff --git a/common/rfb/SSecurityVeNCrypt.cxx b/common/rfb/SSecurityVeNCrypt.cxx
index 7c13749..ce6c71b 100644
--- a/common/rfb/SSecurityVeNCrypt.cxx
+++ b/common/rfb/SSecurityVeNCrypt.cxx
@@ -55,6 +55,8 @@ SSecurityVeNCrypt::SSecurityVeNCrypt(SecurityServer *sec) : security(sec)
SSecurityVeNCrypt::~SSecurityVeNCrypt()
{
+ delete ssecurity;
+
if (subTypes) {
delete [] subTypes;
subTypes = NULL;

23
tigervnc-fix-checknowait-logic-in-ssecurityplain.patch Normal file
View File

@ -0,0 +1,23 @@
From 9801c5efcf8c1774d9c807ebd5d27ac7049ad993 Mon Sep 17 00:00:00 2001
From: Michal Srb <michalsrb@gmail.com>
Date: Wed, 29 Mar 2017 17:00:30 +0300
Subject: Fix checkNoWait logic in SSecurityPlain.
Currently it proceeds only if there aren't enough data in queue and then it blocks waiting.
Also the required amount to receive from network is (ulen + plen), not (ulen + plen + 2).
This allowed not authenticated clients to deny service to everyone.
diff --git a/common/rfb/SSecurityPlain.cxx b/common/rfb/SSecurityPlain.cxx
index f5a5cc7..0531549 100644
--- a/common/rfb/SSecurityPlain.cxx
+++ b/common/rfb/SSecurityPlain.cxx
@@ -92,7 +92,7 @@ bool SSecurityPlain::processMsg(SConnection* sc)
}
if (state == 1) {
- if (is->checkNoWait(ulen + plen + 2))
+ if (!is->checkNoWait(ulen + plen))
return false;
state = 2;
pw = new char[plen + 1];

23
tigervnc-fix-crash-from-integer-overflow.patch Normal file
View File

@ -0,0 +1,23 @@
From bf3bdac082978ca32895a4b6a123016094905689 Mon Sep 17 00:00:00 2001
From: Michal Srb <michalsrb@gmail.com>
Date: Mon, 27 Mar 2017 13:37:11 +0300
Subject: Fix crash from integer overflow in SMsgReader::readClientCutText
The length sent by client is U32, but is converted into int. If it was bigger than 0x7fffffff the resulting int is negative, it passes the check against maxCutText and later throws std::bad_alloc from CharArray which takes down the whole server.
All the Streaming API deals with lengths in ints, so we can't tell it to skip that big amount of data. And it is not realistic to expect more than 2GB of clipboard data anyway. So lets just throw rdr::Exception that will disconnect this client and keep the server alive.
diff --git a/common/rfb/SMsgReader.cxx b/common/rfb/SMsgReader.cxx
index 89c9a8f..3c08fd6 100644
--- a/common/rfb/SMsgReader.cxx
+++ b/common/rfb/SMsgReader.cxx
@@ -200,6 +200,9 @@ void SMsgReader::readClientCutText()
{
is->skip(3);
int len = is->readU32();
+ if (len < 0) {
+ throw Exception("Cut text too long.");
+ }
if (len > maxCutText) {
is->skip(len);
vlog.error("Cut text too long (%d bytes) - ignoring", len);

48
tigervnc-limit-max-username-password-size-in-ssecurityplain.patch Normal file
View File

@ -0,0 +1,48 @@
From 62197c89e98be47a174074e4c7429c57767a4929 Mon Sep 17 00:00:00 2001
From: Michal Srb <michalsrb@gmail.com>
Date: Wed, 29 Mar 2017 17:05:45 +0300
Subject: Limit max username/password size in SSecurityPlain.
Setting the limit to 1024 which should be still more than enough.
Unlimited ulen and plen can cause various security problems:
* Overflow in `is->checkNoWait(ulen + plen)` causing it to contine when there is not enough data and then wait forever.
* Overflow in `new char[plen + 1]` that would allocate zero sized array which succeeds but returns pointer that should not be written into.
* Allocation failure in `new char[plen + 1]` from trying to allocate too much and crashing the whole server.
All those issues can be triggered by a client before authentication.
diff --git a/common/rfb/SSecurityPlain.cxx b/common/rfb/SSecurityPlain.cxx
index 0531549..fc9dff2 100644
--- a/common/rfb/SSecurityPlain.cxx
+++ b/common/rfb/SSecurityPlain.cxx
@@ -86,8 +86,15 @@ bool SSecurityPlain::processMsg(SConnection* sc)
if (state == 0) {
if (!is->checkNoWait(8))
return false;
+
ulen = is->readU32();
+ if (ulen > MaxSaneUsernameLength)
+ throw AuthFailureException("Too long username");
+
plen = is->readU32();
+ if (plen > MaxSanePasswordLength)
+ throw AuthFailureException("Too long password");
+
state = 1;
}
diff --git a/common/rfb/SSecurityPlain.h b/common/rfb/SSecurityPlain.h
index 080fcd5..2c08c24 100644
--- a/common/rfb/SSecurityPlain.h
+++ b/common/rfb/SSecurityPlain.h
@@ -54,6 +54,9 @@ namespace rfb {
PasswordValidator* valid;
unsigned int ulen, plen, state;
CharArray username;
+
+ static const unsigned int MaxSaneUsernameLength = 1024;
+ static const unsigned int MaxSanePasswordLength = 1024;
};
}

34
tigervnc-prevent-double-free-by-crafted-fences.patch Normal file
View File

@ -0,0 +1,34 @@
From f3afa24da144409a3c3a0e35913112583d987671 Mon Sep 17 00:00:00 2001
From: Michal Srb <michalsrb@gmail.com>
Date: Mon, 27 Mar 2017 19:02:15 +0300
Subject: Prevent double free by crafted fences.
If client sent fence with some data, followed by fence with no data (length 0), the original fence data were freed, but the pointer kept pointing at them. Sending one more fence would attempt to free them again.
diff --git a/common/rfb/SMsgWriter.cxx b/common/rfb/SMsgWriter.cxx
index cf3264e..bc3f439 100644
--- a/common/rfb/SMsgWriter.cxx
+++ b/common/rfb/SMsgWriter.cxx
@@ -101,7 +101,9 @@ void SMsgWriter::writeFence(rdr::U32 flags, unsigned len, const char data[])
os->writeU32(flags);
os->writeU8(len);
- os->writeBytes(data, len);
+
+ if (len > 0)
+ os->writeBytes(data, len);
endMsg();
}
diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx
index 0a2ca33..d2206f9 100644
--- a/common/rfb/VNCSConnectionST.cxx
+++ b/common/rfb/VNCSConnectionST.cxx
@@ -666,6 +666,7 @@ void VNCSConnectionST::fence(rdr::U32 flags, unsigned len, const char data[])
fenceFlags = flags & (fenceFlagBlockBefore | fenceFlagBlockAfter | fenceFlagSyncNext);
fenceDataLen = len;
delete [] fenceData;
+ fenceData = NULL;
if (len > 0) {
fenceData = new char[len];
memcpy(fenceData, data, len);

139
tigervnc-prevent-leak-of-securityserver-and-clientserver.patch Normal file
View File

@ -0,0 +1,139 @@
From dccb5f7d776e93863ae10bbff56a45c523c6eeb0 Mon Sep 17 00:00:00 2001
From: Michal Srb <michalsrb@gmail.com>
Date: Mon, 27 Mar 2017 13:55:46 +0300
Subject: Prevent leak of SecurityServer and ClientServer.
They are created in SConnection's and CConnection's constructors but never destroyed.
There is no reason for the indirection, so lets make them direct members.
diff --git a/common/rfb/CConnection.cxx b/common/rfb/CConnection.cxx
index 2020418..88befd5 100644
--- a/common/rfb/CConnection.cxx
+++ b/common/rfb/CConnection.cxx
@@ -44,7 +44,6 @@ CConnection::CConnection()
state_(RFBSTATE_UNINITIALISED), useProtocol3_3(false),
framebuffer(NULL), decoder(this)
{
- security = new SecurityClient();
}
CConnection::~CConnection()
@@ -167,7 +166,7 @@ void CConnection::processSecurityTypesMsg()
int secType = secTypeInvalid;
std::list<rdr::U8> secTypes;
- secTypes = security->GetEnabledSecTypes();
+ secTypes = security.GetEnabledSecTypes();
if (cp.isVersion(3,3)) {
@@ -235,7 +234,7 @@ void CConnection::processSecurityTypesMsg()
}
state_ = RFBSTATE_SECURITY;
- csecurity = security->GetCSecurity(secType);
+ csecurity = security.GetCSecurity(secType);
processSecurityMsg();
}
diff --git a/common/rfb/CConnection.h b/common/rfb/CConnection.h
index 799a9c2..e0a000f 100644
--- a/common/rfb/CConnection.h
+++ b/common/rfb/CConnection.h
@@ -26,6 +26,7 @@
#include <rfb/CMsgHandler.h>
#include <rfb/DecodeManager.h>
+#include <rfb/SecurityClient.h>
#include <rfb/util.h>
namespace rfb {
@@ -34,7 +35,6 @@ namespace rfb {
class CMsgWriter;
class CSecurity;
class IdentityVerifier;
- class SecurityClient;
class CConnection : public CMsgHandler {
public:
@@ -148,7 +148,7 @@ namespace rfb {
stateEnum state() { return state_; }
CSecurity *csecurity;
- SecurityClient *security;
+ SecurityClient security;
protected:
void setState(stateEnum s) { state_ = s; }
diff --git a/common/rfb/SConnection.cxx b/common/rfb/SConnection.cxx
index 17ef4d9..85cc6e8 100644
--- a/common/rfb/SConnection.cxx
+++ b/common/rfb/SConnection.cxx
@@ -51,7 +51,7 @@ const SConnection::AccessRights SConnection::AccessFull = 0xffff;
SConnection::SConnection()
: readyForSetColourMapEntries(false),
is(0), os(0), reader_(0), writer_(0),
- security(0), ssecurity(0), state_(RFBSTATE_UNINITIALISED),
+ ssecurity(0), state_(RFBSTATE_UNINITIALISED),
preferredEncoding(encodingRaw)
{
defaultMajorVersion = 3;
@@ -60,8 +60,6 @@ SConnection::SConnection()
defaultMinorVersion = 3;
cp.setVersion(defaultMajorVersion, defaultMinorVersion);
-
- security = new SecurityServer();
}
SConnection::~SConnection()
@@ -142,7 +140,7 @@ void SConnection::processVersionMsg()
std::list<rdr::U8> secTypes;
std::list<rdr::U8>::iterator i;
- secTypes = security->GetEnabledSecTypes();
+ secTypes = security.GetEnabledSecTypes();
if (cp.isVersion(3,3)) {
@@ -161,7 +159,7 @@ void SConnection::processVersionMsg()
os->writeU32(*i);
if (*i == secTypeNone) os->flush();
state_ = RFBSTATE_SECURITY;
- ssecurity = security->GetSSecurity(*i);
+ ssecurity = security.GetSSecurity(*i);
processSecurityMsg();
return;
}
@@ -193,7 +191,7 @@ void SConnection::processSecurityType(int secType)
std::list<rdr::U8> secTypes;
std::list<rdr::U8>::iterator i;
- secTypes = security->GetEnabledSecTypes();
+ secTypes = security.GetEnabledSecTypes();
for (i=secTypes.begin(); i!=secTypes.end(); i++)
if (*i == secType) break;
if (i == secTypes.end())
@@ -204,7 +202,7 @@ void SConnection::processSecurityType(int secType)
try {
state_ = RFBSTATE_SECURITY;
- ssecurity = security->GetSSecurity(secType);
+ ssecurity = security.GetSSecurity(secType);
} catch (rdr::Exception& e) {
throwConnFailedException(e.str());
}
diff --git a/common/rfb/SConnection.h b/common/rfb/SConnection.h
index b43cf08..63dc314 100644
--- a/common/rfb/SConnection.h
+++ b/common/rfb/SConnection.h
@@ -196,7 +196,7 @@ namespace rfb {
rdr::OutStream* os;
SMsgReader* reader_;
SMsgWriter* writer_;
- SecurityServer *security;
+ SecurityServer security;
SSecurity* ssecurity;
stateEnum state_;
rdr::S32 preferredEncoding;

39
tigervnc.spec
View File

@ -1,6 +1,6 @@
Name: tigervnc
Version: 1.7.1
Release: 2%{?dist}
Release: 3%{?dist}
Summary: A TigerVNC remote display system
%global _hardened_build 1
@ -57,6 +57,23 @@ Patch9: tigervnc-shebang.patch
Patch14: tigervnc-xstartup.patch
Patch18: tigervnc-utilize-system-crypto-policies.patch
# Upstream fixes for CVEs
# Bug 1438694 - (CVE-2017-7392) CVE-2017-7392 tigervnc: SSecurityVeNCrypt memory leak
Patch50: tigervnc-delete-underlying-ssecurity.patch
# Bug 1438697 - (CVE-2017-7393) CVE-2017-7393 tigervnc: Double free via crafted fences
Patch51: tigervnc-prevent-double-free-by-crafted-fences.patch
# Bug 1438700 - (CVE-2017-7394) CVE-2017-7394 tigervnc: Server crash via long filenames
Patch52: tigervnc-limit-max-username-password-size-in-ssecurityplain.patch
# Bug 1438701 - (CVE-2017-7395) CVE-2017-7395 tigervnc: Integer overflow in SMsgReader::readClientCutText
Patch53: tigervnc-fix-crash-from-integer-overflow.patch
# Bug 1438703 - (CVE-2017-7396) CVE-2017-7396 tigervnc: SecurityServer and ClientServer memory leaks
Patch54: tigervnc-prevent-leak-of-securityserver-and-clientserver.patch
# Other important fixes
Patch60: tigervnc-avoid-leaking-shared-memory.patch
Patch61: tigervnc-be-more-restrictive-with-shared-memory-mode-bits.patch
Patch62: tigervnc-fix-checknowait-logic-in-ssecurityplain.patch
# This is tigervnc-%%{version}/unix/xserver116.patch rebased on the latest xorg
Patch100: tigervnc-xserver119.patch
@ -175,6 +192,21 @@ popd
# Utilize system-wide crypto policies
%patch18 -p1 -b .utilize-system-crypto-policies.patch
# Bug 1438694 - (CVE-2017-7392) CVE-2017-7392 tigervnc: SSecurityVeNCrypt memory leak
%patch50 -p1 -b .delete-underlying-ssecurity.patch
# Bug 1438697 - (CVE-2017-7393) CVE-2017-7393 tigervnc: Double free via crafted fences
%patch51 -p1 -b .prevent-double-free-by-crafted-fences.patch
# Bug 1438700 - (CVE-2017-7394) CVE-2017-7394 tigervnc: Server crash via long filenames
%patch52 -p1 -b .limit-max-username-password-size-in-ssecurityplain.patch
# Bug 1438701 - (CVE-2017-7395) CVE-2017-7395 tigervnc: Integer overflow in SMsgReader::readClientCutText
%patch53 -p1 -b .fix-crash-from-integer-overflow.patch
# Bug 1438703 - (CVE-2017-7396) CVE-2017-7396 tigervnc: SecurityServer and ClientServer memory leaks
%patch54 -p1 -b .prevent-leak-of-securityserver-and-clientserver.patch
# Other important fixes
%patch60 -p1 -b .avoid-leaking-shared-memory.patch
%patch61 -p1 -b .be-more-restrictive-with-shared-memory-mode-bits.patch
%patch62 -p1 -b .fix-checknowait-logic-in-ssecurityplain.patch
%build
%ifarch sparcv9 sparc64 s390 s390x
@ -329,6 +361,11 @@ fi
%{_datadir}/icons/hicolor/*/apps/*
%changelog
* Tue Apr 04 2017 Jan Grulich <jgrulich@redhat.com> - 1.7.1-3
- Bug 1438704 - CVE-2017-7392 CVE-2017-7393 CVE-2017-7394
CVE-2017-7395 CVE-2017-7396 tigervnc: various flaws
+ other upstream related fixes
* Sat Feb 11 2017 Fedora Release Engineering <releng@fedoraproject.org> - 1.7.1-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_26_Mass_Rebuild