From f26d6d57ded526ecba5fe1de4e1a69753fae0a7a Mon Sep 17 00:00:00 2001 From: pajlada Date: Sun, 15 Jun 2025 15:46:24 +0200 Subject: [PATCH] fix: don't post contents of unknown commands to chat (#6272) --- CHANGELOG.md | 1 + src/common/ChatterinoSetting.hpp | 2 +- src/providers/twitch/TwitchChannel.cpp | 33 ++++++++++++++- src/providers/twitch/TwitchChannel.hpp | 18 ++++++++ src/providers/twitch/TwitchIrcServer.cpp | 20 +-------- src/singletons/Settings.cpp | 15 +++++++ src/singletons/Settings.hpp | 3 ++ tests/CMakeLists.txt | 1 + tests/src/TwitchChannel.cpp | 53 ++++++++++++++++++++++++ 9 files changed, 126 insertions(+), 20 deletions(-) create mode 100644 tests/src/TwitchChannel.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index cf601edd..11b6e492 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/common/ChatterinoSetting.hpp b/src/common/ChatterinoSetting.hpp index fb4a536c..84242838 100644 --- a/src/common/ChatterinoSetting.hpp +++ b/src/common/ChatterinoSetting.hpp @@ -130,7 +130,7 @@ public: return this->getEnum(); } - Enum getEnum() + Enum getEnum() const { return qmagicenum::enumCast(this->getValue(), qmagicenum::CASE_INSENSITIVE) diff --git a/src/providers/twitch/TwitchChannel.cpp b/src/providers/twitch/TwitchChannel.cpp index f5d52b18..7ae56e5f 100644 --- a/src/providers/twitch/TwitchChannel.cpp +++ b/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 = diff --git a/src/providers/twitch/TwitchChannel.hpp b/src/providers/twitch/TwitchChannel.hpp index f93f719c..62316875 100644 --- a/src/providers/twitch/TwitchChannel.hpp +++ b/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: diff --git a/src/providers/twitch/TwitchIrcServer.cpp b/src/providers/twitch/TwitchIrcServer.cpp index 1e0f7501..3458cb60 100644 --- a/src/providers/twitch/TwitchIrcServer.cpp +++ b/src/providers/twitch/TwitchIrcServer.cpp @@ -129,22 +129,6 @@ void sendHelixMessage(const std::shared_ptr &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); } diff --git a/src/singletons/Settings.cpp b/src/singletons/Settings.cpp index c2a7a1a8..e052906f 100644 --- a/src/singletons/Settings.cpp +++ b/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); diff --git a/src/singletons/Settings.hpp b/src/singletons/Settings.hpp index 965f6a21..6d268488 100644 --- a/src/singletons/Settings.hpp +++ b/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}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 02e948b5..2aff24c8 100644 --- a/tests/CMakeLists.txt +++ b/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 diff --git a/tests/src/TwitchChannel.cpp b/tests/src/TwitchChannel.cpp new file mode 100644 index 00000000..c36f6cae --- /dev/null +++ b/tests/src/TwitchChannel.cpp @@ -0,0 +1,53 @@ +#include "providers/twitch/TwitchChannel.hpp" + +#include "Test.hpp" + +#include + +#include + +namespace chatterino::detail { + +TEST(TwitchChannelDetail_isUnknownCommand, good) +{ + // clang-format off + std::vector 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 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