diff options
Diffstat (limited to 'deskutils/kdeconnect-kde/files/patch-CVE-2020-26164')
| -rw-r--r-- | deskutils/kdeconnect-kde/files/patch-CVE-2020-26164 | 377 |
1 files changed, 377 insertions, 0 deletions
diff --git a/deskutils/kdeconnect-kde/files/patch-CVE-2020-26164 b/deskutils/kdeconnect-kde/files/patch-CVE-2020-26164 new file mode 100644 index 000000000000..267f90b078d2 --- /dev/null +++ b/deskutils/kdeconnect-kde/files/patch-CVE-2020-26164 @@ -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 + |