From 264e97625abe2e0334f97de17f6ffb52582888ab Mon Sep 17 00:00:00 2001

From: Albert Astals Cid <aacid@kde.org>

Date: Wed, 10 May 2017 10:06:07 +0200

Subject: Verify that whoever is calling us is actually who he says he is

CVE-2017-8422

---

 kdecore/auth/AuthBackend.cpp                       |  5 ++++

 kdecore/auth/AuthBackend.h                         |  7 ++++++

 kdecore/auth/backends/dbus/DBusHelperProxy.cpp     | 27 ++++++++++++++++++++--

 kdecore/auth/backends/dbus/DBusHelperProxy.h       |  6 ++++-

 .../auth/backends/policykit/PolicyKitBackend.cpp   |  5 ++++

 kdecore/auth/backends/policykit/PolicyKitBackend.h |  1 +

 kdecore/auth/backends/polkit-1/Polkit1Backend.cpp  |  5 ++++

 kdecore/auth/backends/polkit-1/Polkit1Backend.h    |  1 +

 8 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/kdecore/auth/AuthBackend.cpp b/kdecore/auth/AuthBackend.cpp

index c953b81..0ba4650 100644

--- a/kdecore/auth/AuthBackend.cpp

+++ b/kdecore/auth/AuthBackend.cpp

@@ -54,6 +54,11 @@ void AuthBackend::setCapabilities(AuthBackend::Capabilities capabilities)

     d->capabilities = capabilities;

 }

+AuthBackend::ExtraCallerIDVerificationMethod AuthBackend::extraCallerIDVerificationMethod() const

+{

+    return NoExtraCallerIDVerificationMethod;

+}

+

 bool AuthBackend::actionExists(const QString& action)

 {

     Q_UNUSED(action);

diff --git a/kdecore/auth/AuthBackend.h b/kdecore/auth/AuthBackend.h

index a86732e..6f4b1bc 100644

--- a/kdecore/auth/AuthBackend.h

+++ b/kdecore/auth/AuthBackend.h

@@ -43,6 +43,12 @@ public:

     };

     Q_DECLARE_FLAGS(Capabilities, Capability)

+    enum ExtraCallerIDVerificationMethod {

+        NoExtraCallerIDVerificationMethod,

+        VerifyAgainstDBusServiceName,

+        VerifyAgainstDBusServicePid,

+    };

+

     AuthBackend();

     virtual ~AuthBackend();

     virtual void setupAction(const QString &action) = 0;

@@ -50,6 +56,7 @@ public:

     virtual Action::AuthStatus authorizeAction(const QString &action) = 0;

     virtual Action::AuthStatus actionStatus(const QString &action) = 0;

     virtual QByteArray callerID() const = 0;

+    virtual ExtraCallerIDVerificationMethod extraCallerIDVerificationMethod() const;

     virtual bool isCallerAuthorized(const QString &action, QByteArray callerID) = 0;

     virtual bool actionExists(const QString &action);

diff --git a/kdecore/auth/backends/dbus/DBusHelperProxy.cpp b/kdecore/auth/backends/dbus/DBusHelperProxy.cpp

index 9557a0f..ca59f1c 100644

--- a/kdecore/auth/backends/dbus/DBusHelperProxy.cpp

+++ b/kdecore/auth/backends/dbus/DBusHelperProxy.cpp

@@ -271,6 +271,29 @@ void DBusHelperProxy::performActions(QByteArray blob, const QByteArray &callerID

     }

 }

+bool DBusHelperProxy::isCallerAuthorized(const QString &action, const QByteArray &callerID)

+{

+    // Check the caller is really who it says it is

+    switch (BackendsManager::authBackend()->extraCallerIDVerificationMethod()) {

+        case AuthBackend::NoExtraCallerIDVerificationMethod:

+        break;

+

+        case AuthBackend::VerifyAgainstDBusServiceName:

+            if (message().service().toUtf8() != callerID) {

+                return false;

+            }

+        break;

+

+        case AuthBackend::VerifyAgainstDBusServicePid:

+            if (connection().interface()->servicePid(message().service()).value() != callerID.toUInt()) {

+                return false;

+            }

+        break;

+    }

+

+    return BackendsManager::authBackend()->isCallerAuthorized(action, callerID);

+}

+

 QByteArray DBusHelperProxy::performAction(const QString &action, const QByteArray &callerID, QByteArray arguments)

 {

     if (!responder) {

@@ -295,7 +318,7 @@ QByteArray DBusHelperProxy::performAction(const QString &action, const QByteArra

     QTimer *timer = responder->property("__KAuth_Helper_Shutdown_Timer").value<QTimer*>();

     timer->stop();

-    if (BackendsManager::authBackend()->isCallerAuthorized(action, callerID)) {

+    if (isCallerAuthorized(action, callerID)) {

         QString slotname = action;

         if (slotname.startsWith(m_name + QLatin1Char('.'))) {

             slotname = slotname.right(slotname.length() - m_name.length() - 1);

@@ -338,7 +361,7 @@ uint DBusHelperProxy::authorizeAction(const QString& action, const QByteArray& c

     QTimer *timer = responder->property("__KAuth_Helper_Shutdown_Timer").value<QTimer*>();

     timer->stop();

-    if (BackendsManager::authBackend()->isCallerAuthorized(action, callerID)) {

+    if (isCallerAuthorized(action, callerID)) {

         retVal = static_cast<uint>(Action::Authorized);

     } else {

         retVal = static_cast<uint>(Action::Denied);

diff --git a/kdecore/auth/backends/dbus/DBusHelperProxy.h b/kdecore/auth/backends/dbus/DBusHelperProxy.h

index 455cf51..264f6cc 100644

--- a/kdecore/auth/backends/dbus/DBusHelperProxy.h

+++ b/kdecore/auth/backends/dbus/DBusHelperProxy.h

@@ -21,6 +21,7 @@

 #ifndef DBUS_HELPER_PROXY_H

 #define DBUS_HELPER_PROXY_H

+#include <QDBusContext>

 #include <QVariant>

 #include "HelperProxy.h"

 #include "kauthactionreply.h"

@@ -28,7 +29,7 @@

 namespace KAuth

 {

-class DBusHelperProxy : public HelperProxy

+class DBusHelperProxy : public HelperProxy, protected QDBusContext

 {

     Q_OBJECT

     Q_INTERFACES(KAuth::HelperProxy)

@@ -73,6 +74,9 @@ signals:

 private slots:

     void remoteSignalReceived(int type, const QString &action, QByteArray blob);

+

+private:

+    bool isCallerAuthorized(const QString &action, const QByteArray &callerID);

 };

 } // namespace Auth

diff --git a/kdecore/auth/backends/policykit/PolicyKitBackend.cpp b/kdecore/auth/backends/policykit/PolicyKitBackend.cpp

index 3be97f2..9d041d1 100644

--- a/kdecore/auth/backends/policykit/PolicyKitBackend.cpp

+++ b/kdecore/auth/backends/policykit/PolicyKitBackend.cpp

@@ -78,6 +78,11 @@ QByteArray PolicyKitBackend::callerID() const

     return a;

 }

+AuthBackend::ExtraCallerIDVerificationMethod Polkit1Backend::extraCallerIDVerificationMethod() const

+{

+    return VerifyAgainstDBusServicePid;

+}

+

 bool PolicyKitBackend::isCallerAuthorized(const QString &action, QByteArray callerID)

 {

     QDataStream s(&callerID, QIODevice::ReadOnly);

diff --git a/kdecore/auth/backends/policykit/PolicyKitBackend.h b/kdecore/auth/backends/policykit/PolicyKitBackend.h

index 7154e93..0d3d8f9 100644

--- a/kdecore/auth/backends/policykit/PolicyKitBackend.h

+++ b/kdecore/auth/backends/policykit/PolicyKitBackend.h

@@ -40,6 +40,7 @@ public:

     virtual Action::AuthStatus authorizeAction(const QString&);

     virtual Action::AuthStatus actionStatus(const QString&);

     virtual QByteArray callerID() const;

+    virtual ExtraCallerIDVerificationMethod extraCallerIDVerificationMethod() const;

     virtual bool isCallerAuthorized(const QString &action, QByteArray callerID);

 private Q_SLOTS:

diff --git a/kdecore/auth/backends/polkit-1/Polkit1Backend.cpp b/kdecore/auth/backends/polkit-1/Polkit1Backend.cpp

index 732d2cb..63c0e1e 100644

--- a/kdecore/auth/backends/polkit-1/Polkit1Backend.cpp

+++ b/kdecore/auth/backends/polkit-1/Polkit1Backend.cpp

@@ -163,6 +163,11 @@ QByteArray Polkit1Backend::callerID() const

     return QDBusConnection::systemBus().baseService().toUtf8();

 }

+AuthBackend::ExtraCallerIDVerificationMethod Polkit1Backend::extraCallerIDVerificationMethod() const

+{

+    return VerifyAgainstDBusServiceName;

+}

+

 bool Polkit1Backend::isCallerAuthorized(const QString &action, QByteArray callerID)

 {

     PolkitQt1::SystemBusNameSubject subject(QString::fromUtf8(callerID));

diff --git a/kdecore/auth/backends/polkit-1/Polkit1Backend.h b/kdecore/auth/backends/polkit-1/Polkit1Backend.h

index 18ed1a2..d579da2 100644

--- a/kdecore/auth/backends/polkit-1/Polkit1Backend.h

+++ b/kdecore/auth/backends/polkit-1/Polkit1Backend.h

@@ -48,6 +48,7 @@ public:

     virtual Action::AuthStatus authorizeAction(const QString&);

     virtual Action::AuthStatus actionStatus(const QString&);

     virtual QByteArray callerID() const;

+    virtual ExtraCallerIDVerificationMethod extraCallerIDVerificationMethod() const;

     virtual bool isCallerAuthorized(const QString &action, QByteArray callerID);

     virtual bool actionExists(const QString& action);

--

cgit v0.11.2