[Servatrice] Match trusted_sources by address, not substring (#7582)

trusted_sources was checked with QString::contains. A dual-stack listener
reports a local proxy as ::ffff:127.0.0.1, which never matches the default
"127.0.0.1,::1". Behind nginx the forwarded client IP header was then
ignored, every websocket user shared the proxy's address, and logins
failed with "too many concurrent connections" once max_users_per_address
was reached.

Add isTrustedSource(), which parses each entry as a QHostAddress and
compares exact addresses with IPv4-mapped addresses normalized, and use it
at all four call sites. The ban command also read server/trusted_sources
instead of the documented security/trusted_sources; it now uses the same
getter. Entries that are not IP addresses are logged at startup since they
no longer match anything.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

servatrice/CMakeLists.txt

@@ -18,6 +18,7 @@ set(servatrice_SOURCES
     src/settingscache.cpp
     src/isl_interface.cpp
     src/signalhandler.cpp
+    src/trusted_sources.cpp
     ${VERSION_STRING_CPP}
     src/smtpclient.cpp
     src/smtp/qxthmac.cpp

servatrice/servatrice.ini.example

@@ -465,7 +465,8 @@ legacy_backup_ttl_days=180
 
 ; You may want to allow an unlimited number of users from a trusted source. This setting can contain a
 ; comma-separed list of IP addresses which will allow an unlimited number of connections from each of the
-; IP addresses listed (ignoring the max_users_per_address). Default is "127.0.0.1,::1"; example: "192.73.233.244,81.4.100.74"
+; IP addresses listed (ignoring the max_users_per_address). Entries must be exact IP addresses; hostnames, prefixes
+; and subnets are ignored. Default is "127.0.0.1,::1"; example: "192.73.233.244,81.4.100.74"
 trusted_sources="127.0.0.1,::1"
 
 ; Servatrice can avoid users from flooding rooms with large number of messages in an interval of time.

servatrice/src/servatrice.cpp

@@ -33,6 +33,7 @@
 #include "serversocketinterface.h"
 #include "settingscache.h"
 #include "smtpclient.h"
+#include "trusted_sources.h"
 
 #include <QDateTime>
 #include <QDebug>
@@ -324,6 +325,14 @@ bool Servatrice::initServer()
         qDebug() << "Maximum websocket user limit:" << getMaxWebSocketUserLimit();
     }
 
+    // trusted_sources entries are matched as exact addresses; older configs may hold hostnames,
+    // prefixes or subnets that substring matching used to accept and that now never match.
+    for (const QString &entry : getTrustedSources().split(',', Qt::SkipEmptyParts)) {
+        if (QHostAddress(entry.trimmed()).isNull()) {
+            qWarning() << "security/trusted_sources entry is not an IP address and will be ignored:" << entry.trimmed();
+        }
+    }
+
     qDebug() << "Accept registered users only:" << getRegOnlyServerEnabled();
     qDebug() << "Registration enabled:" << getRegistrationEnabled();
     if (getRegistrationEnabled()) {
@@ -1324,6 +1333,11 @@ int Servatrice::getMaxUsersPerAddress() const
     return settingsCache->value("security/max_users_per_address", 4).toInt();
 }
 
+QString Servatrice::getTrustedSources() const
+{
+    return settingsCache->value("security/trusted_sources", "127.0.0.1,::1").toString();
+}
+
 int Servatrice::getMessageCountingInterval() const
 {
     return settingsCache->value("security/message_counting_interval", 10).toInt();
@@ -1538,8 +1552,7 @@ bool Servatrice::isRateLimitExempt(const QString &ipAddress) const
     // Mirrors the max_users_per_address bypass in initSession: trusted sources
     // (e.g. 127.0.0.1 for Webatrice behind local nginx, where every websocket
     // client shares one address) are not subject to per-address limits.
-    const QString trustedSources = settingsCache->value("security/trusted_sources", "127.0.0.1,::1").toString();
-    return trustedSources.contains(ipAddress, Qt::CaseInsensitive);
+    return isTrustedSource(getTrustedSources(), ipAddress);
 }
 
 void Servatrice::clearFailedLogins(const QString &ipAddress, const QString &userName)

servatrice/src/servatrice.h

@@ -321,6 +321,8 @@ public:
     int getDeckShareMaxDecksPerShare() const;
     int getDeckShareMaxSharesPerDay() const;
     int getMaxUsersPerAddress() const;
+    /// Raw security/trusted_sources list; match addresses against it with isTrustedSource().
+    QString getTrustedSources() const;
     int getMessageCountingInterval() const override;
     int getMaxMessageCountPerInterval() const override;
     int getMaxMessageSizePerInterval() const override;

servatrice/src/serversocketinterface.cpp

@@ -42,6 +42,7 @@
 #include "server_logger.h"
 #include "server_protocolhandler.h"
 #include "settingscache.h"
+#include "trusted_sources.h"
 #include "version_string.h"
 
 #include <QChar>
@@ -220,8 +221,7 @@ bool AbstractServerSocketInterface::initSession()
     delete identSe;
 
     // allow unlimited number of connections from the trusted sources
-    QString trustedSources = settingsCache->value("security/trusted_sources", "127.0.0.1,::1").toString();
-    if (trustedSources.contains(getAddress(), Qt::CaseInsensitive)) {
+    if (isTrustedSource(servatrice->getTrustedSources(), getPeerAddress())) {
         return true;
     }
 
@@ -1908,9 +1908,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdBanFromServer(const Com
     if (amountRemove != 0) {
         removeSaidMessages(userName, amountRemove);
     }
-    QString trustedSources = settingsCache->value("server/trusted_sources", "127.0.0.1,::1").toString();
     int minutes = cmd.minutes();
-    if (trustedSources.contains(address, Qt::CaseInsensitive)) {
+    if (isTrustedSource(servatrice->getTrustedSources(), address)) {
         address = "";
     }
 
@@ -4560,8 +4559,7 @@ void WebsocketServerSocketInterface::initConnection(void *_socket)
     if (websocketIPHeader.length() > 0 && socket->request().hasRawHeader(websocketIPHeader)) {
         // Only trust the forwarded header if the connection itself comes from a trusted proxy,
         // otherwise a client could spoof its address to bypass bans and rate limits.
-        QString trustedSources = settingsCache->value("security/trusted_sources", "127.0.0.1,::1").toString();
-        if (trustedSources.contains(socket->peerAddress().toString(), Qt::CaseInsensitive)) {
+        if (isTrustedSource(servatrice->getTrustedSources(), socket->peerAddress())) {
             QString header(socket->request().rawHeader(websocketIPHeader));
             QHostAddress parsed(header);
             if (!parsed.isNull()) {

servatrice/src/trusted_sources.cpp

@@ -0,0 +1,23 @@
+#include "trusted_sources.h"
+
+#include <QStringList>
+
+bool isTrustedSource(const QString &trustedSources, const QHostAddress &address)
+{
+    if (address.isNull()) {
+        return false;
+    }
+    const QStringList entries = trustedSources.split(',', Qt::SkipEmptyParts);
+    for (const QString &entry : entries) {
+        const QHostAddress trusted(entry.trimmed());
+        if (!trusted.isNull() && trusted.isEqual(address, QHostAddress::ConvertV4MappedToIPv4)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+bool isTrustedSource(const QString &trustedSources, const QString &address)
+{
+    return isTrustedSource(trustedSources, QHostAddress(address.trimmed()));
+}

servatrice/src/trusted_sources.h

@@ -0,0 +1,20 @@
+#ifndef TRUSTED_SOURCES_H
+#define TRUSTED_SOURCES_H
+
+#include <QHostAddress>
+#include <QString>
+
+/**
+ * @brief True if @p address is listed in the comma-separated @p trustedSources (security/trusted_sources).
+ *
+ * Entries are parsed as addresses and compared exactly, never as substrings, so
+ * 27.0.0.1 does not match 127.0.0.1. IPv4-mapped IPv6 addresses compare equal to
+ * their IPv4 form: a dual-stack listener reports a local proxy as ::ffff:127.0.0.1,
+ * which matches a 127.0.0.1 entry. Malformed entries and null addresses never match.
+ */
+bool isTrustedSource(const QString &trustedSources, const QHostAddress &address);
+
+/** @brief Overload for an address in string form; an unparsable string never matches. */
+bool isTrustedSource(const QString &trustedSources, const QString &address);
+
+#endif

tests/CMakeLists.txt

@@ -208,6 +208,17 @@ cockatrice_add_test(
   LABELS
   server
 )
+cockatrice_add_test(
+  NAME
+  trusted_sources_test
+  SOURCES
+  ${CMAKE_SOURCE_DIR}/servatrice/src/trusted_sources.cpp
+  trusted_sources_test.cpp
+  INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/servatrice/src
+  LABELS
+  server
+)
 cockatrice_add_test(
   NAME
   server_card_counter_test

tests/trusted_sources_test.cpp

@@ -0,0 +1,58 @@
+#include "trusted_sources.h"
+
+#include "gtest/gtest.h"
+#include <QHostAddress>
+
+namespace
+{
+
+const QString kDefault = QStringLiteral("127.0.0.1,::1");
+
+TEST(TrustedSourcesTest, MatchesListedAddresses)
+{
+    ASSERT_TRUE(isTrustedSource(kDefault, QHostAddress("127.0.0.1")));
+    ASSERT_TRUE(isTrustedSource(kDefault, QHostAddress("::1")));
+    ASSERT_FALSE(isTrustedSource(kDefault, QHostAddress("10.0.0.1")));
+}
+
+TEST(TrustedSourcesTest, MatchesIpv4MappedPeerAgainstPlainIpv4Entry)
+{
+    // A dual-stack listener reports a local IPv4 proxy as ::ffff:127.0.0.1;
+    // it must still match the default 127.0.0.1 entry.
+    ASSERT_TRUE(isTrustedSource(kDefault, QHostAddress("::ffff:127.0.0.1")));
+}
+
+TEST(TrustedSourcesTest, MatchesPlainIpv4PeerAgainstMappedEntry)
+{
+    ASSERT_TRUE(isTrustedSource("::ffff:192.0.2.7", QHostAddress("192.0.2.7")));
+}
+
+TEST(TrustedSourcesTest, DoesNotMatchSubstrings)
+{
+    // Substring matching would let 27.0.0.1 or 0.0.1 pass as 127.0.0.1.
+    ASSERT_FALSE(isTrustedSource(kDefault, QHostAddress("27.0.0.1")));
+    ASSERT_FALSE(isTrustedSource("192.0.2.10", QHostAddress("192.0.2.1")));
+    ASSERT_FALSE(isTrustedSource(kDefault, QHostAddress(":")));
+}
+
+TEST(TrustedSourcesTest, ToleratesWhitespaceAndCase)
+{
+    ASSERT_TRUE(isTrustedSource(" 192.0.2.7 , 2001:DB8::1 ", QHostAddress("2001:db8::1")));
+    ASSERT_TRUE(isTrustedSource(" 192.0.2.7 , 2001:DB8::1 ", QHostAddress("192.0.2.7")));
+}
+
+TEST(TrustedSourcesTest, IgnoresMalformedEntriesAndNullAddresses)
+{
+    ASSERT_FALSE(isTrustedSource("not-an-ip,,", QHostAddress("127.0.0.1")));
+    ASSERT_FALSE(isTrustedSource("", QHostAddress("127.0.0.1")));
+    ASSERT_FALSE(isTrustedSource(kDefault, QHostAddress()));
+}
+
+TEST(TrustedSourcesTest, StringOverloadParsesAddress)
+{
+    ASSERT_TRUE(isTrustedSource(kDefault, QStringLiteral("::ffff:127.0.0.1")));
+    ASSERT_FALSE(isTrustedSource(kDefault, QStringLiteral("27.0.0.1")));
+    ASSERT_FALSE(isTrustedSource(kDefault, QString()));
+}
+
+} // namespace