From 90d3c5b95145c1fa326d1d6d9fa5bcd7b3dedc4c Mon Sep 17 00:00:00 2001
From: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@qt.io>
Date: Thu, 27 Apr 2023 16:29:01 +0200
Subject: QDBusServer: Fix potential crash when private pointer is null

Check that the private pointer is not null before attempting
to dereference it. This can happen, for example, when
a QDBusServer instance was constructed with an empty string
as address. Attempting to destroy an object constructed
this way was causing a segmentation fault on Linux.

Add a test case that attempts to construct a QDBusServer
object with an empty string as address to check that this
does not cause a segmentation fault anymore.

Pick-to: 6.5 6.2
Change-Id: I5fe63134026e2a9f509b61d452285891b1ec624d
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/dbus/qdbusserver.cpp                           | 33 +++++++++++++++-------
 .../dbus/qdbusconnection/tst_qdbusconnection.cpp   |  5 ++++
 .../dbus/qdbusconnection/tst_qdbusconnection.h     |  2 ++
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/src/dbus/qdbusserver.cpp b/src/dbus/qdbusserver.cpp
index 21d422ac9e..2efc61a671 100644
--- a/src/dbus/qdbusserver.cpp
+++ b/src/dbus/qdbusserver.cpp
@@ -39,6 +39,8 @@ QDBusServer::QDBusServer(const QString &address, QObject *parent)
         return;
 
     emit instance->serverRequested(address, this);
+    Q_ASSERT(d != nullptr);
+
     QObject::connect(d, SIGNAL(newServerConnection(QDBusConnectionPrivate*)),
                      this, SLOT(_q_newConnection(QDBusConnectionPrivate*)), Qt::QueuedConnection);
 }
@@ -66,6 +68,8 @@ QDBusServer::QDBusServer(QObject *parent)
         return;
 
     emit instance->serverRequested(address, this);
+    Q_ASSERT(d != nullptr);
+
     QObject::connect(d, SIGNAL(newServerConnection(QDBusConnectionPrivate*)),
                      this, SLOT(_q_newConnection(QDBusConnectionPrivate*)), Qt::QueuedConnection);
 }
@@ -75,17 +79,20 @@ QDBusServer::QDBusServer(QObject *parent)
 */
 QDBusServer::~QDBusServer()
 {
-    QMutex *managerMutex = nullptr;
-    if (QDBusConnectionManager::instance())
-        managerMutex = &QDBusConnectionManager::instance()->mutex;
-    QMutexLocker locker(managerMutex);
+    if (!d)
+        return;
+
+    auto manager = QDBusConnectionManager::instance();
+    if (!manager)
+        return;
+
+    QMutexLocker locker(&manager->mutex);
     QWriteLocker writeLocker(&d->lock);
-    if (QDBusConnectionManager::instance()) {
-        for (const QString &name : std::as_const(d->serverConnectionNames))
-            QDBusConnectionManager::instance()->removeConnection(name);
-        d->serverConnectionNames.clear();
-        locker.unlock();
-    }
+    for (const QString &name : std::as_const(d->serverConnectionNames))
+        manager->removeConnection(name);
+    d->serverConnectionNames.clear();
+    locker.unlock();
+
     d->serverObject = nullptr;
     d->ref.storeRelaxed(0);
     d->deleteLater();
@@ -138,6 +145,9 @@ QString QDBusServer::address() const
 */
 void QDBusServer::setAnonymousAuthenticationAllowed(bool value)
 {
+    if (!d)
+        return;
+
     d->anonymousAuthenticationAllowed = value;
 }
 
@@ -150,6 +160,9 @@ void QDBusServer::setAnonymousAuthenticationAllowed(bool value)
 */
 bool QDBusServer::isAnonymousAuthenticationAllowed() const
 {
+    if (!d)
+        return false;
+
     return d->anonymousAuthenticationAllowed;
 }
 
diff --git a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
index 09ce947ccc..42e898eda7 100644
--- a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
+++ b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
@@ -1412,6 +1412,11 @@ void tst_QDBusConnection::pendingCallWhenDisconnected()
 #endif
 }
 
+void tst_QDBusConnection::emptyServerAddress()
+{
+    QDBusServer server({}, nullptr);
+}
+
 QString MyObject::path;
 QString MyObjectWithoutInterface::path;
 QString MyObjectWithoutInterface::interface;
diff --git a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h
index 058f4e8115..0aec2aa0bd 100644
--- a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h
+++ b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h
@@ -115,6 +115,8 @@ private slots:
     void callVirtualObjectLocal();
     void pendingCallWhenDisconnected();
 
+    void emptyServerAddress();
+
 public:
     QString serviceName() const { return "org.qtproject.Qt.Autotests.QDBusConnection"; }
     bool callMethod(const QDBusConnection &conn, const QString &path);
-- 
cgit v1.2.3