fix: IPC security review fixes

Author: NickVs2015

ipc/ipc.h

@@ -61,8 +61,6 @@ inline QStringList sanitizeArguments(PermittedProcess proc, const QStringList &a
 
     switch (proc) {
     case OpenVPN: {
-        // Whitelist only args actually used by the client:
-        // --config <path>, --management <host> <port>, --management-client
         namedArgs["--config"] = [](const QString& v) { return !v.isEmpty(); };
         namedArgs["--management"] = [](const QString& v) { return !v.isEmpty(); };
         namedArgs["--management-client"] = nullptr;
@@ -73,17 +71,6 @@ inline QStringList sanitizeArguments(PermittedProcess proc, const QStringList &a
         });
         break;
     }
-    case Wireguard: {
-        // Whitelist only subcommand + config file path (wg-quick up/down <conf>)
-        if (args.size() == 2) {
-            const QString &sub = args[0];
-            if (sub == QStringLiteral("up") || sub == QStringLiteral("down")) {
-                return args;
-            }
-        }
-        qWarning() << "IPC: blocked unexpected WireGuard arguments";
-        return {};
-    }
     case Tun2Socks:
         namedArgs["-device"] = [](const QString& v) { return v.startsWith("tun://"); };
         namedArgs["-proxy"] = [](const QString& v) { return v.startsWith("socks5://"); };

ipc/ipcserver.cpp

@@ -22,27 +22,6 @@
     #include "tapcontroller_win.h"
 #endif
 
-#ifdef Q_OS_LINUX
-#include <sys/socket.h>
-#include <sys/types.h>
-
-extern uid_t g_allowedUid;
-extern bool g_allowedUidSet;
-
-static bool checkPrivPeerCredentials(QLocalSocket *socket) {
-    struct ucred cred{};
-    socklen_t len = sizeof(cred);
-    if (getsockopt(socket->socketDescriptor(), SOL_SOCKET, SO_PEERCRED, &cred, &len) != 0) {
-        qWarning() << "IpcServer: SO_PEERCRED failed, rejecting privileged process connection";
-        return false;
-    }
-    if (cred.uid == 0) return true;
-    if (g_allowedUidSet && cred.uid == g_allowedUid) return true;
-    qWarning() << "IpcServer: rejected privileged process connection from unauthorized UID" << cred.uid;
-    return false;
-}
-#endif
-
 
 IpcServer::IpcServer(QObject *parent) : IpcInterfaceSource(parent)
 {
@@ -70,13 +49,6 @@ int IpcServer::createPrivilegedProcess()
     QObject::connect(pd.localServer.data(), &QLocalServer::newConnection, this, [pd]() {
         qDebug() << "IpcServer new connection";
         QLocalSocket *conn = pd.localServer->nextPendingConnection();
-#ifdef Q_OS_LINUX
-        if (!checkPrivPeerCredentials(conn)) {
-            conn->close();
-            conn->deleteLater();
-            return;
-        }
-#endif
         if (pd.serverNode) {
             pd.serverNode->addHostSideConnection(conn);
             pd.serverNode->enableRemoting(pd.ipcProcess.data());

service/server/killswitch.cpp

@@ -189,7 +189,7 @@ bool KillSwitch::disableAllTraffic() {
 
 bool KillSwitch::resetAllowedRange(const QStringList &ranges) {
 
-#ifdef Q_OS_LINUX
+#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS)
     if (!std::all_of(ranges.cbegin(), ranges.cend(), isValidIpOrCidr)) {
         qCritical() << "IPC: invalid IP/CIDR in ranges, rejecting resetAllowedRange";
         return false;
@@ -220,7 +220,7 @@ bool KillSwitch::resetAllowedRange(const QStringList &ranges) {
 }
 
 bool KillSwitch::addAllowedRange(const QStringList &ranges) {
-#ifdef Q_OS_LINUX
+#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS)
     if (!std::all_of(ranges.cbegin(), ranges.cend(), isValidIpOrCidr)) {
         qCritical() << "IPC: invalid IP/CIDR in ranges, rejecting addAllowedRange";
         return false;
@@ -323,6 +323,15 @@ bool KillSwitch::enableKillSwitch(const QJsonObject &configStr, int vpnAdapterIn
     bool allowMarkedXray = 0;
     QStringList allownets;
     QStringList blocknets;
+    QStringList allowedDnsServers;
+    for (const QJsonValue &dns : configStr.value(amnezia::configKey::allowedDnsServers).toArray()) {
+        if (!dns.isString()) break;
+        const QString dnsStr = dns.toString();
+        if (isValidIpOrCidr(dnsStr))
+            allowedDnsServers.append(dnsStr);
+        else if (!dnsStr.isEmpty())
+            qWarning() << "IPC: rejected invalid allowedDnsServer:" << dnsStr;
+    }
 
     if (splitTunnelType == 0) {
         blockAll = true;
@@ -387,18 +396,7 @@ bool KillSwitch::enableKillSwitch(const QJsonObject &configStr, int vpnAdapterIn
 
     dnsServers.append("127.0.0.1");
     dnsServers.append("127.0.0.53");
-
-
-    for (auto dns : configStr.value(amnezia::configKey::allowedDnsServers).toArray()) {
-        if (!dns.isString()) {
-            break;
-        }
-        const QString dnsStr = dns.toString();
-        if (isValidIpOrCidr(dnsStr))
-            dnsServers.append(dnsStr);
-        else if (!dnsStr.isEmpty())
-            qWarning() << "IPC: rejected invalid allowedDnsServer:" << dnsStr;
-    }
+    dnsServers.append(allowedDnsServers);
 
     LinuxFirewall::updateDNSServers(dnsServers);
     LinuxFirewall::setAnchorEnabled(LinuxFirewall::IPv4, QStringLiteral("320.allowDNS"), true);
@@ -412,6 +410,11 @@ bool KillSwitch::enableKillSwitch(const QJsonObject &configStr, int vpnAdapterIn
         MacOSFirewall::install();
 
     MacOSFirewall::ensureRootAnchorPriority();
+    if (!std::all_of(allownets.cbegin(), allownets.cend(), isValidIpOrCidr) ||
+        !std::all_of(blocknets.cbegin(), blocknets.cend(), isValidIpOrCidr)) {
+        qCritical() << "IPC: invalid IP/CIDR in allownets/blocknets, rejecting enableKillSwitch";
+        return false;
+    }
     MacOSFirewall::setAnchorEnabled(QStringLiteral("000.allowLoopback"), true);
     MacOSFirewall::setAnchorEnabled(QStringLiteral("100.blockAll"), blockAll);
     MacOSFirewall::setAnchorEnabled(QStringLiteral("110.allowNets"), allowNets);
@@ -440,17 +443,8 @@ bool KillSwitch::enableKillSwitch(const QJsonObject &configStr, int vpnAdapterIn
             qWarning() << "IPC: rejected invalid dns2:" << dns2;
     }
 
-    for (auto dns : configStr.value(amnezia::configKey::allowedDnsServers).toArray()) {
-        if (!dns.isString()) {
-            break;
-        }
-        const QString dnsStr = dns.toString();
-        if (isValidIpOrCidr(dnsStr))
-            dnsServers.append(dnsStr);
-        else if (!dnsStr.isEmpty())
-            qWarning() << "IPC: rejected invalid allowedDnsServer:" << dnsStr;
-    }
-    
+    dnsServers.append(allowedDnsServers);
+
     MacOSFirewall::setAnchorEnabled(QStringLiteral("310.blockDNS"), true);
     MacOSFirewall::setAnchorTable(QStringLiteral("310.blockDNS"), true, QStringLiteral("dnsaddr"), dnsServers);
     MacOSFirewall::setAnchorEnabled(QStringLiteral("400.allowPIA"), true);

service/server/localserver.cpp

@@ -17,7 +17,6 @@
     #include "tapcontroller_win.h"
 #endif
 
-
 namespace {
 Logger logger("WgDaemonServer");
 }
@@ -36,8 +35,7 @@ LocalServer::LocalServer(QObject *parent) : QObject(parent),
 
     QObject::connect(m_server.data(), &QLocalServer::newConnection, this, [this]() {
         qDebug() << "LocalServer new connection";
-        QLocalSocket *conn = m_server->nextPendingConnection();
-        m_serverNode.addHostSideConnection(conn);
+        m_serverNode.addHostSideConnection(m_server->nextPendingConnection());
 
         if (!m_isRemotingEnabled) {
             m_isRemotingEnabled = true;