fix: don't post contents of unknown commands to chat (#6272)

CHANGELOG.md

@@ -37,6 +37,7 @@
 - Bugfix: Fixed some minor typos. (#6196)
 - Bugfix: Fixed inconsistent spaces in messages when using fractional scaling. (#6231, #6254)
 - Bugfix: Don't add reply buttons to messages that are invalid reply targets. (#6119)
+- Bugfix: Fixed invalid commands from being forwarded to Helix, making it possible for information to leak (e.g. if you typed `/bann username ban reason` it would be seen by others in chat as `username ban reason`). (#6272)
 - Dev: Mini refactor of Split. (#6148)
 - Dev: Conan will no longer generate a `CMakeUserPresets.json` file. (#6117)
 - Dev: Pass `--force-openssl` when installing from CMake in Qt 6.8+. (#6129)

src/common/ChatterinoSetting.hpp

@@ -130,7 +130,7 @@ public:
         return this->getEnum();
     }
 
-    Enum getEnum()
+    Enum getEnum() const
     {
         return qmagicenum::enumCast<Enum>(this->getValue(),
                                           qmagicenum::CASE_INSENSITIVE)

src/providers/twitch/TwitchChannel.cpp

@@ -61,6 +61,22 @@ namespace chatterino {
 
 using namespace literals;
 
+namespace detail {
+
+bool isUnknownCommand(const QString &text)
+{
+    static QRegularExpression isUnknownCommand(
+        R"(^(?:\.|\/)(?!me\s|\s))", QRegularExpression::CaseInsensitiveOption);
+
+    auto match = isUnknownCommand.match(text);
+
+    return match.hasMatch();
+}
+
+}  // namespace detail
+
+using detail::isUnknownCommand;
+
 namespace {
 #if QT_VERSION < QT_VERSION_CHECK(6, 1, 0)
 const QString MAGIC_MESSAGE_SUFFIX = QString((const char *)u8" \U000E0000");
@@ -91,6 +107,7 @@ constexpr auto MAX_CHATTERS_TO_FETCH = 5000;
 
 // From Twitch docs - expected size for a badge (1x)
 constexpr QSize BASE_BADGE_SIZE(18, 18);
+
 }  // namespace
 
 TwitchChannel::TwitchChannel(const QString &name)
@@ -794,6 +811,13 @@ void TwitchChannel::sendMessage(const QString &message)
         return;
     }
 
+    if (getSettings()->shouldSendHelixChat() && isUnknownCommand(parsedMessage))
+    {
+        this->addSystemMessage(QString("%1 is not a known command.")
+                                   .arg(parsedMessage.split(' ').first()));
+        return;
+    }
+
     bool messageSent = false;
     this->sendMessageSignal.invoke(this->getName(), parsedMessage, messageSent);
     this->updateSevenTVActivity();
@@ -828,6 +852,13 @@ void TwitchChannel::sendReply(const QString &message, const QString &replyId)
         return;
     }
 
+    if (getSettings()->shouldSendHelixChat() && isUnknownCommand(parsedMessage))
+    {
+        this->addSystemMessage(QString("%1 is not a known command.")
+                                   .arg(parsedMessage.split(' ').first()));
+        return;
+    }
+
     bool messageSent = false;
     this->sendReplySignal.invoke(this->getName(), parsedMessage, replyId,
                                  messageSent);
@@ -1452,7 +1483,7 @@ void TwitchChannel::loadRecentMessagesReconnect()
     int limit = getSettings()->twitchMessageHistoryLimit.getValue();
     if (this->lastConnectedAt_.has_value())
     {
-        // calculate how many messages could have occured
+        // calculate how many messages could have occurred
         // while we were not connected to the channel
         // assuming a maximum of 10 messages per second
         const auto secondsSinceDisconnect =

src/providers/twitch/TwitchChannel.hpp

@@ -64,6 +64,24 @@ class TwitchAccount;
 
 const int MAX_QUEUED_REDEMPTIONS = 16;
 
+namespace detail {
+
+/// isUnknownCommand checks if the given text contains a command that should not be forwarded to Twitch
+///
+/// "/ hello" should be allowed
+/// ". hello" should be allowed
+/// "/me hello" should be allowed
+/// ".me hello" should be allowed
+/// "/mebadcommand hello" should NOT be allowed
+/// ".mebadcommand hello" should NOT be allowed
+/// "/badcommand hello" should NOT be allowed
+/// "/badcommand hello" should NOT be allowed
+/// ".@badcommand hello" should NOT be allowed
+/// ".@badcommand hello" should NOT be allowed
+bool isUnknownCommand(const QString &text);
+
+}  // namespace detail
+
 class TwitchChannel final : public Channel, public ChannelChatters
 {
 public:

src/providers/twitch/TwitchIrcServer.cpp

@@ -129,22 +129,6 @@ void sendHelixMessage(const std::shared_ptr<TwitchChannel> &channel,
         });
 }
 
-/// Returns true if chat messages should be sent over Helix
-bool shouldSendHelixChat()
-{
-    switch (getSettings()->chatSendProtocol)
-    {
-        case ChatSendProtocol::Helix:
-            return true;
-        case ChatSendProtocol::Default:
-        case ChatSendProtocol::IRC:
-            return false;
-        default:
-            assert(false && "Invalid chat protocol value");
-            return false;
-    }
-}
-
 }  // namespace
 
 namespace chatterino {
@@ -762,7 +746,7 @@ void TwitchIrcServer::onMessageSendRequested(
         return;
     }
 
-    if (shouldSendHelixChat())
+    if (getSettings()->shouldSendHelixChat())
     {
         sendHelixMessage(channel, message);
     }
@@ -786,7 +770,7 @@ void TwitchIrcServer::onReplySendRequested(
         return;
     }
 
-    if (shouldSendHelixChat())
+    if (getSettings()->shouldSendHelixChat())
     {
         sendHelixMessage(channel, message, replyId);
     }

src/singletons/Settings.cpp

@@ -283,6 +283,21 @@ void Settings::disableSave()
     this->disableSaving = true;
 }
 
+bool Settings::shouldSendHelixChat() const
+{
+    switch (this->chatSendProtocol.getEnum())
+    {
+        case ChatSendProtocol::Helix:
+            return true;
+        case ChatSendProtocol::Default:
+        case ChatSendProtocol::IRC:
+            return false;
+        default:
+            assert(false && "Invalid chat protocol value");
+            return false;
+    }
+}
+
 float Settings::getClampedUiScale() const
 {
     return std::clamp(this->uiScale.getValue(), 0.2F, 10.F);

src/singletons/Settings.hpp

@@ -145,6 +145,9 @@ public:
 
     void disableSave();
 
+    /// Returns true if chat messages should be sent over Helix
+    bool shouldSendHelixChat() const;
+
     FloatSetting uiScale = {"/appearance/uiScale2", 1};
     BoolSetting windowTopMost = {"/appearance/windowAlwaysOnTop", false};
 

tests/CMakeLists.txt

@@ -56,6 +56,7 @@ set(test_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/src/WebSocketPool.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/NativeMessaging.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/ImageUploader.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/src/TwitchChannel.cpp
 
     ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.hpp

tests/src/TwitchChannel.cpp

@@ -0,0 +1,53 @@
+#include "providers/twitch/TwitchChannel.hpp"
+
+#include "Test.hpp"
+
+#include <QString>
+
+#include <vector>
+
+namespace chatterino::detail {
+
+TEST(TwitchChannelDetail_isUnknownCommand, good)
+{
+    // clang-format off
+    std::vector<QString> cases{
+        "/me hello",
+        ".me hello",
+        "/ hello",
+        ". hello",
+        "/ /hello",
+        ". .hello",
+        "/ .hello",
+        ". /hello",
+    };
+    // clang-format on
+
+    for (const auto &input : cases)
+    {
+        ASSERT_FALSE(isUnknownCommand(input))
+            << input << " should not be considered an unknown command";
+    }
+}
+
+TEST(TwitchChannelDetail_isUnknownCommand, bad)
+{
+    // clang-format off
+    std::vector<QString> cases{
+        "/badcommand",
+        ".badcommand",
+        "/badcommand hello",
+        ".badcommand hello",
+        "/@badcommand hello",
+        ".@badcommand hello",
+    };
+    // clang-format on
+
+    for (const auto &input : cases)
+    {
+        ASSERT_TRUE(isUnknownCommand(input))
+            << input << " should be considered an unknown command";
+    }
+}
+
+}  // namespace chatterino::detail