cpet/freebsd-ports
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

MFH: r551357

Browse Source 
deskutils/kdeconnect-kde: add upstreams mitigations for CVE-2020-26164

From https://kde.org/info/security/advisory-20201002-1.txt:
	Solution
	========

	KDE Connect 20.08.2 patches several code paths that could result in a DoS.
	You can apply these patches on top of 20.08.1:
	f183b5447b
	b279c52101
	d35b88c1b2
	b496e66899
	5310eae85d
	721ba9faaf
	ae58b9dec4
	66c768aa9e
	85b691e40f
	48180b4655

Security:	CVE-2020-26164

Approved by:	ports-secteam (joneum)
This commit is contained in:
Tobias C. Berner 2020-10-04 06:56:15 +00:00
parent 48679794ba
commit 68f834a8b2
Notes: svn2git 2021-03-31 03:12:20 +00:00 
svn path=/branches/2020Q4/; revision=551362
3 changed files with 387 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
deskutils/kdeconnect-kde/Makefile
View File

@ -2,6 +2,7 @@
PORTNAME= kdeconnect-kde
DISTVERSION= ${KDE_APPLICATIONS_VERSION}
PORTREVISION= 1
CATEGORIES= deskutils kde kde-applications
MAINTAINER= kde@FreeBSD.org

377
deskutils/kdeconnect-kde/files/patch-CVE-2020-26164 Normal file
View File

@ -0,0 +1,377 @@
KDE Connect 20.08.2 patches several code paths that could result in a DoS.
You can apply these patches on top of 20.08.1:
https://invent.kde.org/network/kdeconnect-kde/-/commit/f183b5447bad47655c21af87214579f03bf3a163
https://invent.kde.org/network/kdeconnect-kde/-/commit/b279c52101d3f7cc30a26086d58de0b5f1c547fa
https://invent.kde.org/network/kdeconnect-kde/-/commit/d35b88c1b25fe13715f9170f18674d476ca9acdc
https://invent.kde.org/network/kdeconnect-kde/-/commit/b496e66899e5bc9547b6537a7f44ab44dd0aaf38
https://invent.kde.org/network/kdeconnect-kde/-/commit/5310eae85dbdf92fba30375238a2481f2e34943e
https://invent.kde.org/network/kdeconnect-kde/-/commit/721ba9faafb79aac73973410ee1dd3624ded97a5
https://invent.kde.org/network/kdeconnect-kde/-/commit/ae58b9dec49c809b85b5404cee17946116f8a706
https://invent.kde.org/network/kdeconnect-kde/-/commit/66c768aa9e7fba30b119c8b801efd49ed1270b0a
https://invent.kde.org/network/kdeconnect-kde/-/commit/85b691e40f525e22ca5cc4ebe79c361d71d7dc05
https://invent.kde.org/network/kdeconnect-kde/-/commit/48180b46552d40729a36b7431e97bbe2b5379306
From 6a3aa96fc0fa8a6f8d92afa2c603a71db061482f Mon Sep 17 00:00:00 2001
From: Albert Vaca Cintora <albertvaka@gmail.com>
Date: Thu, 24 Sep 2020 16:59:22 +0200
Subject: [PATCH] Do not ignore SSL errors, except for self-signed cert errors.
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
Do not leak the local user in the device name.
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
Fix use after free in LanLinkProvider::connectError()
If QSslSocket::connectToHost() hasn't finished running.
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
Limit identity packets to 8KiB
Healthy identity packages shouldn't be that big and we don't want to
allow systems around us to send us ever humongous packages that will
just leave us without any memory.
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
Do not let lanlink connections stay open for long without authenticating
If there's no information received, close the socket to try again.
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
Don't brute-force reading the socket
The package will arrive eventually, and dataReceived will be emitted.
Otherwise we just end up calling dataReceived to no end.
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
Limit number of connected sockets from unpaired devices
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
Do not remember more than a few identity packets at a time
To prevent the kdeconnect process from using too much memory.
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
Limit the ports we try to connect to to the port range of KDE Connect
So we can't trigger connections to other services.
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
Do not replace connections for a given deviceId if the certs have changed
Thanks Matthias Gerstner <mgerstner@suse.de> for reporting this.
---
core/backends/lan/landevicelink.cpp | 5 ++
core/backends/lan/landevicelink.h | 1 +
core/backends/lan/lanlinkprovider.cpp | 79 ++++++++++++++++++++++----
core/backends/lan/socketlinereader.cpp | 8 ---
core/kdeconnectconfig.cpp | 8 +--
tests/testsocketlinereader.cpp | 31 +++++++++-
6 files changed, 103 insertions(+), 29 deletions(-)
diff --git core/backends/lan/landevicelink.cpp core/backends/lan/landevicelink.cpp
index 8a65fb92..41af6f0e 100644
--- core/backends/lan/landevicelink.cpp
+++ core/backends/lan/landevicelink.cpp
@@ -192,3 +192,8 @@ bool LanDeviceLink::linkShouldBeKeptAlive() {
//return (mConnectionSource == ConnectionStarted::Remotely || pairStatus() == Paired);
}
+
+QSslCertificate LanDeviceLink::certificate() const
+{
+ return m_socketLineReader->peerCertificate();
+}
diff --git core/backends/lan/landevicelink.h core/backends/lan/landevicelink.h
index 28f63db2..485c58b5 100644
--- core/backends/lan/landevicelink.h
+++ core/backends/lan/landevicelink.h
@@ -56,6 +56,7 @@ public:
bool linkShouldBeKeptAlive() override;
QHostAddress hostAddress() const;
+ QSslCertificate certificate() const;
private Q_SLOTS:
void dataReceived();
diff --git core/backends/lan/lanlinkprovider.cpp core/backends/lan/lanlinkprovider.cpp
index d9a7d8fa..372cdc8f 100644
--- core/backends/lan/lanlinkprovider.cpp
+++ core/backends/lan/lanlinkprovider.cpp
@@ -46,6 +46,9 @@
#define MIN_VERSION_WITH_SSL_SUPPORT 6
+static const int MAX_UNPAIRED_CONNECTIONS = 42;
+static const int MAX_REMEMBERED_IDENTITY_PACKETS = 42;
+
LanLinkProvider::LanLinkProvider(
bool testMode,
quint16 udpBroadcastPort,
@@ -220,9 +223,20 @@ void LanLinkProvider::udpBroadcastReceived()
}
int tcpPort = receivedPacket->get<int>(QStringLiteral("tcpPort"));
+ if (tcpPort < MIN_TCP_PORT || tcpPort > MAX_TCP_PORT) {
+ qCDebug(KDECONNECT_CORE) << "TCP port outside of kdeconnect's range";
+ delete receivedPacket;
+ continue;
+ }
//qCDebug(KDECONNECT_CORE) << "Received Udp identity packet from" << sender << " asking for a tcp connection on port " << tcpPort;
+ if (m_receivedIdentityPackets.size() > MAX_REMEMBERED_IDENTITY_PACKETS) {
+ qCWarning(KDECONNECT_CORE) << "Too many remembered identities, ignoring" << receivedPacket->get<QString>(QStringLiteral("deviceId")) << "received via UDP";
+ delete receivedPacket;
+ continue;
+ }
+
QSslSocket* socket = new QSslSocket(this);
socket->setProxy(QNetworkProxy::NoProxy);
m_receivedIdentityPackets[socket].np = receivedPacket;
@@ -252,7 +266,7 @@ void LanLinkProvider::connectError(QAbstractSocket::SocketError socketError)
//The socket we created didn't work, and we didn't manage
//to create a LanDeviceLink from it, deleting everything.
delete m_receivedIdentityPackets.take(socket).np;
- delete socket;
+ socket->deleteLater();
}
//We received a UDP packet and answered by connecting to them by TCP. This gets called on a successful connection.
@@ -297,9 +311,7 @@ void LanLinkProvider::tcpSocketConnected()
connect(socket, &QSslSocket::encrypted, this, &LanLinkProvider::encrypted);
- if (isDeviceTrusted) {
- connect(socket, QOverload<const QList<QSslError> &>::of(&QSslSocket::sslErrors), this, &LanLinkProvider::sslErrors);
- }
+ connect(socket, QOverload<const QList<QSslError> &>::of(&QSslSocket::sslErrors), this, &LanLinkProvider::sslErrors);
socket->startServerEncryption();
@@ -326,8 +338,6 @@ void LanLinkProvider::encrypted()
QSslSocket* socket = qobject_cast<QSslSocket*>(sender());
if (!socket) return;
- // TODO delete me?
- disconnect(socket, QOverload<const QList<QSslError> &>::of(&QSslSocket::sslErrors), this, &LanLinkProvider::sslErrors);
Q_ASSERT(socket->mode() != QSslSocket::UnencryptedMode);
LanDeviceLink::ConnectionStarted connectionOrigin = (socket->mode() == QSslSocket::SslClientMode)? LanDeviceLink::Locally : LanDeviceLink::Remotely;
@@ -335,6 +345,12 @@ void LanLinkProvider::encrypted()
NetworkPacket* receivedPacket = m_receivedIdentityPackets[socket].np;
const QString& deviceId = receivedPacket->get<QString>(QStringLiteral("deviceId"));
+ if (m_links.contains(deviceId) && m_links[deviceId]->certificate() != socket->peerCertificate()) {
+ socket->disconnectFromHost();
+ qCWarning(KDECONNECT_CORE) << "Got connection for the same deviceId but certificates don't match. Ignoring " << deviceId;
+ return;
+ }
+
addLink(deviceId, socket, receivedPacket, connectionOrigin);
// Copied from tcpSocketConnected slot, now delete received packet
@@ -346,14 +362,20 @@ void LanLinkProvider::sslErrors(const QList<QSslError>& errors)
QSslSocket* socket = qobject_cast<QSslSocket*>(sender());
if (!socket) return;
- qCDebug(KDECONNECT_CORE) << "Failing due to " << errors;
- Device* device = Daemon::instance()->getDevice(socket->peerVerifyName());
- if (device) {
- device->unpair();
+ bool fatal = false;
+ for (const QSslError& error : errors) {
+ if (error.error() != QSslError::SelfSignedCertificate) {
+ qCCritical(KDECONNECT_CORE) << "Disconnecting due to fatal SSL Error: " << error;
+ fatal = true;
+ } else {
+ qCDebug(KDECONNECT_CORE) << "Ignoring self-signed cert error";
+ }
}
- delete m_receivedIdentityPackets.take(socket).np;
- // Socket disconnects itself on ssl error and will be deleted by deleteLater slot, no need to delete manually
+ if (fatal) {
+ socket->disconnectFromHost();
+ delete m_receivedIdentityPackets.take(socket).np;
+ }
}
//I'm the new device and this is the answer to my UDP identity packet (no data received yet). They are connecting to us through TCP, and they should send an identity.
@@ -372,6 +394,16 @@ void LanLinkProvider::newConnection()
connect(socket, &QIODevice::readyRead,
this, &LanLinkProvider::dataReceived);
+ QTimer* timer = new QTimer(socket);
+ timer->setSingleShot(true);
+ timer->setInterval(1000);
+ connect(socket, &QSslSocket::encrypted,
+ timer, &QObject::deleteLater);
+ connect(timer, &QTimer::timeout, socket, [socket] {
+ qCWarning(KDECONNECT_CORE) << "LanLinkProvider/newConnection: Host timed out without sending any identity." << socket->peerAddress();
+ socket->disconnectFromHost();
+ });
+ timer->start();
}
}
@@ -379,6 +411,14 @@ void LanLinkProvider::newConnection()
void LanLinkProvider::dataReceived()
{
QSslSocket* socket = qobject_cast<QSslSocket*>(sender());
+ //the size here is arbitrary and is now at 8192 bytes. It needs to be considerably long as it includes the capabilities but there needs to be a limit
+ //Tested between my systems and I get around 2000 per identity package.
+ if (socket->bytesAvailable() > 8192) {
+ qCWarning(KDECONNECT_CORE) << "LanLinkProvider/newConnection: Suspiciously long identity package received. Closing connection." << socket->peerAddress() << socket->bytesAvailable();
+ socket->disconnectFromHost();
+ return;
+ }
+
#if QT_VERSION < QT_VERSION_CHECK(5,7,0)
if (!socket->canReadLine())
return;
@@ -413,6 +453,12 @@ void LanLinkProvider::dataReceived()
return;
}
+ if (m_receivedIdentityPackets.size() > MAX_REMEMBERED_IDENTITY_PACKETS) {
+ qCWarning(KDECONNECT_CORE) << "Too many remembered identities, ignoring" << np->get<QString>(QStringLiteral("deviceId")) << "received via TCP";
+ delete np;
+ return;
+ }
+
// Needed in "encrypted" if ssl is used, similar to "tcpSocketConnected"
m_receivedIdentityPackets[socket].np = np;
@@ -535,6 +581,15 @@ void LanLinkProvider::addLink(const QString& deviceId, QSslSocket* socket, Netwo
deviceLink->reset(socket, connectionOrigin);
} else {
deviceLink = new LanDeviceLink(deviceId, this, socket, connectionOrigin);
+ // Socket disconnection will now be handled by LanDeviceLink
+ disconnect(socket, &QAbstractSocket::disconnected, socket, &QObject::deleteLater);
+ bool isDeviceTrusted = KdeConnectConfig::instance().trustedDevices().contains(deviceId);
+ if (!isDeviceTrusted && m_links.size() > MAX_UNPAIRED_CONNECTIONS) {
+ qCWarning(KDECONNECT_CORE) << "Too many unpaired devices to remember them all. Ignoring " << deviceId;
+ socket->disconnectFromHost();
+ socket->deleteLater();
+ return;
+ }
connect(deviceLink, &QObject::destroyed, this, &LanLinkProvider::deviceLinkDestroyed);
m_links[deviceId] = deviceLink;
if (m_pairingHandlers.contains(deviceId)) {
diff --git core/backends/lan/socketlinereader.cpp core/backends/lan/socketlinereader.cpp
index f67fdf3f..da77052a 100644
--- core/backends/lan/socketlinereader.cpp
+++ core/backends/lan/socketlinereader.cpp
@@ -38,14 +38,6 @@ void SocketLineReader::dataReceived()
}
}
- //If we still have things to read from the socket, call dataReceived again
- //We do this manually because we do not trust readyRead to be emitted again
- //So we call this method again just in case.
- if (m_socket->bytesAvailable() > 0) {
- QMetaObject::invokeMethod(this, "dataReceived", Qt::QueuedConnection);
- return;
- }
-
//If we have any packets, tell it to the world.
if (!m_packets.isEmpty()) {
Q_EMIT readyRead();
diff --git core/kdeconnectconfig.cpp core/kdeconnectconfig.cpp
index 91719303..a8dbcf5c 100644
--- core/kdeconnectconfig.cpp
+++ core/kdeconnectconfig.cpp
@@ -90,13 +90,7 @@ KdeConnectConfig::KdeConnectConfig()
QString KdeConnectConfig::name()
{
- QString username;
- #ifdef Q_OS_WIN
- username = QString::fromLatin1(qgetenv("USERNAME"));
- #else
- username = QString::fromLatin1(qgetenv("USER"));
- #endif
- QString defaultName = username + QStringLiteral("@") + QHostInfo::localHostName();
+ QString defaultName = QHostInfo::localHostName();
QString name = d->m_config->value(QStringLiteral("name"), defaultName).toString();
return name;
}
diff --git tests/testsocketlinereader.cpp tests/testsocketlinereader.cpp
index 75584556..b6425b03 100644
--- tests/testsocketlinereader.cpp
+++ tests/testsocketlinereader.cpp
@@ -25,16 +25,19 @@
#include <QProcess>
#include <QEventLoop>
#include <QTimer>
+#include <QSignalSpy>
class TestSocketLineReader : public QObject
{
Q_OBJECT
public Q_SLOTS:
- void initTestCase();
+ void init();
+ void cleanup() { delete m_server; }
void newPacket();
private Q_SLOTS:
void socketLineReader();
+ void badData();
private:
QTimer m_timer;
@@ -45,8 +48,9 @@ private:
SocketLineReader* m_reader;
};
-void TestSocketLineReader::initTestCase()
+void TestSocketLineReader::init()
{
+ m_packets.clear();
m_server = new Server(this);
QVERIFY2(m_server->listen(QHostAddress::LocalHost, 8694), "Failed to create local tcp server");
@@ -97,6 +101,29 @@ void TestSocketLineReader::socketLineReader()
}
}
+void TestSocketLineReader::badData()
+{
+ const QList<QByteArray> dataToSend = { "data1\n", "data" }; //does not end in a \n
+ for (const QByteArray& line : qAsConst(dataToSend)) {
+ m_conn->write(line);
+ }
+ m_conn->flush();
+
+ QSignalSpy spy(m_server, &QTcpServer::newConnection);
+ QVERIFY(m_server->hasPendingConnections() || spy.wait(1000));
+ QSslSocket* sock = m_server->nextPendingConnection();
+
+ QVERIFY2(sock != nullptr, "Could not open a connection to the client");
+
+ m_reader = new SocketLineReader(sock, this);
+ connect(m_reader, &SocketLineReader::readyRead, this, &TestSocketLineReader::newPacket);
+ m_timer.start();
+ m_loop.exec();
+
+ QCOMPARE(m_packets.count(), 1);
+ QCOMPARE(m_packets[0], dataToSend[0]);
+}
+
void TestSocketLineReader::newPacket()
{
if (!m_reader->bytesAvailable()) {
--
2.28.0

18
deskutils/kdeconnect-kde/files/patch-core_backends_lan_lanlinkprovider.cpp
View File

@ -1,10 +1,10 @@
--- core/backends/lan/lanlinkprovider.cpp.orig 2018-05-30 21:41:03 UTC
+++ core/backends/lan/lanlinkprovider.cpp
@@ -196,6 +196,17 @@ void LanLinkProvider::newUdpConnection() //udpBroadcas
int tcpPort = receivedPacket->get<int>(QStringLiteral("tcpPort"));
+ // convert IPv6 addresses of type "v4-mapped" to IPv4
--- core/backends/lan/lanlinkprovider.cpp.cve 2020-10-04 08:10:12.704397000 +0200
+++ core/backends/lan/lanlinkprovider.cpp 2020-10-04 08:12:38.587533000 +0200
@@ -229,6 +229,17 @@
continue;
}
+ // convert IPv6 addresses of type "v4-mapped" to IPv4
+ QHostAddress addr = sender;
+ if (addr.protocol() == QAbstractSocket::IPv6Protocol) {
+ bool success;
@ -16,5 +16,5 @@
+ }
+
//qCDebug(KDECONNECT_CORE) << "Received Udp identity packet from" << sender << " asking for a tcp connection on port " << tcpPort;
QSslSocket* socket = new QSslSocket(this);
if (m_receivedIdentityPackets.size() > MAX_REMEMBERED_IDENTITY_PACKETS) {