From 4743cb3980a26cac8c57c3f392cdd12c4d6f1c10 Mon Sep 17 00:00:00 2001 From: Matt Haynie Date: Sun, 16 Aug 2020 04:04:21 -0700 Subject: [PATCH] Added a more robust log message filtering method that should prevent the users's steam api key from being recorded to the log file or output to the app log in any circumstances. --- tf2_bot_detector/Config/Settings.cpp | 17 +++++-- tf2_bot_detector/Config/Settings.h | 8 +++- tf2_bot_detector/Log.cpp | 59 ++++++++++++++++++++++-- tf2_bot_detector/Log.h | 2 + tf2_bot_detector/MainWindow.cpp | 5 +- tf2_bot_detector/Networking/SteamAPI.cpp | 14 +----- tf2_bot_detector/WorldState.cpp | 16 +++---- 7 files changed, 89 insertions(+), 32 deletions(-) diff --git a/tf2_bot_detector/Config/Settings.cpp b/tf2_bot_detector/Config/Settings.cpp index a6f4df3b..6dd93711 100644 --- a/tf2_bot_detector/Config/Settings.cpp +++ b/tf2_bot_detector/Config/Settings.cpp @@ -131,6 +131,12 @@ namespace tf2_bot_detector } } +void GeneralSettings::SetSteamAPIKey(std::string key) +{ + ILogManager::GetInstance().AddSecret(key, mh::format("", key.size())); + m_SteamAPIKey = std::move(key); +} + void tf2_bot_detector::to_json(nlohmann::json& j, const ProgramUpdateCheckMode& d) { switch (d) @@ -202,14 +208,13 @@ void Settings::LoadFile() if (auto found = json.find("general"); found != json.end()) { - constexpr GeneralSettings DEFAULTS; + static const GeneralSettings DEFAULTS; try_get_to_defaulted(*found, m_LocalSteamIDOverride, "local_steamid_override"); try_get_to_defaulted(*found, m_SleepWhenUnfocused, "sleep_when_unfocused"); try_get_to_defaulted(*found, m_AutoTempMute, "auto_temp_mute", DEFAULTS.m_AutoTempMute); try_get_to_defaulted(*found, m_AllowInternetUsage, "allow_internet_usage"); try_get_to_defaulted(*found, m_ProgramUpdateCheckMode, "program_update_check_mode", DEFAULTS.m_ProgramUpdateCheckMode); - try_get_to_defaulted(*found, m_SteamAPIKey, "steam_api_key"); try_get_to_defaulted(*found, m_AutoLaunchTF2, "auto_launch_tf2", DEFAULTS.m_AutoLaunchTF2); try_get_to_defaulted(*found, m_AutoChatWarnings, "auto_chat_warnings", DEFAULTS.m_AutoChatWarnings); try_get_to_defaulted(*found, m_AutoChatWarningsConnecting, "auto_chat_warnings_connecting", DEFAULTS.m_AutoChatWarningsConnecting); @@ -218,6 +223,12 @@ void Settings::LoadFile() try_get_to_defaulted(*found, m_AutoMark, "auto_mark", DEFAULTS.m_AutoMark); try_get_to_defaulted(*found, m_LazyLoadAPIData, "lazy_load_api_data", DEFAULTS.m_LazyLoadAPIData); + { + std::string apiKey; + try_get_to_defaulted(*found, apiKey, "steam_api_key"); + SetSteamAPIKey(std::move(apiKey)); + } + if (auto foundDir = found->find("steam_dir_override"); foundDir != found->end()) m_SteamDirOverride = foundDir->get(); if (auto foundDir = found->find("tf_game_dir_override"); foundDir != found->end()) @@ -248,7 +259,7 @@ bool Settings::SaveFile() const { "sleep_when_unfocused", m_SleepWhenUnfocused }, { "auto_temp_mute", m_AutoTempMute }, { "program_update_check_mode", m_ProgramUpdateCheckMode }, - { "steam_api_key", m_SteamAPIKey }, + { "steam_api_key", GetSteamAPIKey() }, { "auto_launch_tf2", m_AutoLaunchTF2 }, { "auto_chat_warnings", m_AutoChatWarnings }, { "auto_chat_warnings_connecting", m_AutoChatWarningsConnecting }, diff --git a/tf2_bot_detector/Config/Settings.h b/tf2_bot_detector/Config/Settings.h index 0b8e32dd..ccd2f299 100644 --- a/tf2_bot_detector/Config/Settings.h +++ b/tf2_bot_detector/Config/Settings.h @@ -67,6 +67,12 @@ namespace tf2_bot_detector ProgramUpdateCheckMode m_ProgramUpdateCheckMode = ProgramUpdateCheckMode::Unknown; constexpr auto GetAutoVotekickDelay() const { return std::chrono::duration(m_AutoVotekickDelay); } + + const std::string& GetSteamAPIKey() const { return m_SteamAPIKey; } + void SetSteamAPIKey(std::string key); + + private: + std::string m_SteamAPIKey; }; class Settings final : public AutoDetectedSettings, public GeneralSettings @@ -91,8 +97,6 @@ namespace tf2_bot_detector } m_Unsaved; - std::string m_SteamAPIKey; - std::optional m_AllowInternetUsage; const HTTPClient* GetHTTPClient() const; diff --git a/tf2_bot_detector/Log.cpp b/tf2_bot_detector/Log.cpp index 4efccf3a..321580f6 100644 --- a/tf2_bot_detector/Log.cpp +++ b/tf2_bot_detector/Log.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -32,7 +33,7 @@ namespace LogManager& operator=(const LogManager&) = delete; void Log(std::string msg, const LogMessageColor& color = {}, time_point_t timestamp = clock_t::now()) override; - void LogToStream(const std::string_view& msg, std::ostream& output, time_point_t timestamp = clock_t::now()) const; + void LogToStream(std::string msg, std::ostream& output, time_point_t timestamp = clock_t::now(), bool skipScrub = false) const; const std::filesystem::path& GetFileName() const override { return m_FileName; } cppcoro::generator GetVisibleMsgs() const override; @@ -44,6 +45,8 @@ namespace void CleanupLogFiles() override; + void AddSecret(std::string value, std::string replace) override; + private: std::filesystem::path m_FileName; std::ofstream m_File; @@ -51,6 +54,14 @@ namespace std::deque m_LogMessages; size_t m_VisibleLogMessagesStart = 0; + struct Secret + { + std::string m_Value; + std::string m_Replacement; + }; + std::vector m_Secrets; + void ReplaceSecrets(std::string& str) const; + static constexpr size_t MAX_LOG_MESSAGES = 500; mutable std::recursive_mutex m_ConsoleLogMutex; @@ -120,10 +131,13 @@ LogManager::LogManager() } } -void LogManager::LogToStream(const std::string_view& msg, std::ostream& output, time_point_t timestamp) const +void LogManager::LogToStream(std::string msg, std::ostream& output, time_point_t timestamp, bool skipScrub) const { std::lock_guard lock(m_LogMutex); + if (!skipScrub) + ReplaceSecrets(msg); + tm t = ToTM(timestamp); const auto WriteToStream = [&](std::ostream& str) { @@ -142,7 +156,9 @@ void LogManager::LogToStream(const std::string_view& msg, std::ostream& output, void LogManager::Log(std::string msg, const LogMessageColor& color, time_point_t timestamp) { std::lock_guard lock(m_LogMutex); - LogToStream(msg, m_File, timestamp); + + ReplaceSecrets(msg); + LogToStream(msg, m_File, timestamp, true); m_LogMessages.push_back({ timestamp, std::move(msg), { color.r, color.g, color.b, color.a } }); if (m_LogMessages.size() > MAX_LOG_MESSAGES) @@ -152,6 +168,43 @@ void LogManager::Log(std::string msg, const LogMessageColor& color, time_point_t } } +void LogManager::AddSecret(std::string value, std::string replace) +{ + if (value.empty()) + return; + + std::lock_guard lock(m_LogMutex); + for (auto& scrubbed : m_Secrets) + { + if (scrubbed.m_Value == value) + { + scrubbed.m_Replacement = std::move(replace); + return; + } + } + + m_Secrets.push_back(Secret + { + .m_Value = std::move(value), + .m_Replacement = std::move(replace) + }); +} + +void LogManager::ReplaceSecrets(std::string& msg) const +{ + std::lock_guard lock(m_LogMutex); + for (const auto& scrubbed : m_Secrets) + { + size_t index = msg.find(scrubbed.m_Value); + while (index != msg.npos) + { + msg.erase(index, scrubbed.m_Value.size()); + msg.insert(index, scrubbed.m_Replacement); + index = msg.find(scrubbed.m_Value, index + scrubbed.m_Replacement.size()); + } + } +} + namespace { template diff --git a/tf2_bot_detector/Log.h b/tf2_bot_detector/Log.h index fa5350f0..74673dee 100644 --- a/tf2_bot_detector/Log.h +++ b/tf2_bot_detector/Log.h @@ -67,5 +67,7 @@ namespace tf2_bot_detector virtual void LogConsoleOutput(const std::string_view& consoleOutput) = 0; virtual void CleanupLogFiles() = 0; + + virtual void AddSecret(std::string value, std::string replace) = 0; }; } diff --git a/tf2_bot_detector/MainWindow.cpp b/tf2_bot_detector/MainWindow.cpp index 68c0122d..db26186a 100644 --- a/tf2_bot_detector/MainWindow.cpp +++ b/tf2_bot_detector/MainWindow.cpp @@ -42,7 +42,6 @@ MainWindow::MainWindow() : { m_TextureManager = CreateTextureManager(); - ILogManager::GetInstance().CleanupLogFiles(); m_WorldState.AddConsoleLineListener(this); @@ -964,10 +963,10 @@ void MainWindow::OnDrawSettingsPopup() ImGui::EnabledSwitch(m_Settings.m_AllowInternetUsage.value_or(false), [&](bool enabled) { ImGui::NewLine(); - if (std::string key = m_Settings.m_SteamAPIKey; + if (std::string key = m_Settings.GetSteamAPIKey(); InputTextSteamAPIKey("Steam API Key", key, true)) { - m_Settings.m_SteamAPIKey = key; + m_Settings.SetSteamAPIKey(key); m_Settings.SaveFile(); } ImGui::NewLine(); diff --git a/tf2_bot_detector/Networking/SteamAPI.cpp b/tf2_bot_detector/Networking/SteamAPI.cpp index b8ae42db..f03ccdc8 100644 --- a/tf2_bot_detector/Networking/SteamAPI.cpp +++ b/tf2_bot_detector/Networking/SteamAPI.cpp @@ -36,19 +36,7 @@ namespace auto clientPtr = client.shared_from_this(); return s_SteamAPIThreadPool.add_task([clientPtr, url]() -> SteamAPITask { - static const std::regex s_Regex(R"regex([?&]key=([0-9a-fA-F]*))regex", std::regex::optimize); - { - auto urlCopy = url; - if (std::smatch result; std::regex_search(urlCopy.m_Path, result, s_Regex)) - { - const size_t startPos = result[1].first - urlCopy.m_Path.begin(); - const auto length = result[1].length(); - urlCopy.m_Path.erase(startPos, length); - urlCopy.m_Path.insert(startPos, mh::fmtstr<64>("", length)); - } - - DebugLog("[SteamAPI] HTTP GET "s << urlCopy); - } + DebugLog("[SteamAPI] HTTP GET "s << url); SteamAPITask retVal; retVal.m_RequestURL = url; diff --git a/tf2_bot_detector/WorldState.cpp b/tf2_bot_detector/WorldState.cpp index c5591f7b..c1a9c59f 100644 --- a/tf2_bot_detector/WorldState.cpp +++ b/tf2_bot_detector/WorldState.cpp @@ -42,10 +42,10 @@ void WorldState::Update() void WorldState::UpdateFriends() { if (auto client = m_Settings->GetHTTPClient(); - client && !m_Settings->m_SteamAPIKey.empty() && (clock_t::now() - 5min) > m_LastFriendsUpdate) + client && !m_Settings->GetSteamAPIKey().empty() && (clock_t::now() - 5min) > m_LastFriendsUpdate) { m_LastFriendsUpdate = clock_t::now(); - m_FriendsFuture = SteamAPI::GetFriendList(m_Settings->m_SteamAPIKey, m_Settings->GetLocalSteamID(), *client); + m_FriendsFuture = SteamAPI::GetFriendList(m_Settings->GetSteamAPIKey(), m_Settings->GetLocalSteamID(), *client); } if (mh::is_future_ready(m_FriendsFuture)) @@ -719,13 +719,13 @@ const SteamAPI::TF2PlaytimeResult* WorldState::PlayerExtraData::GetTF2Playtime() { if (!m_TF2PlaytimeFetched) { - if (!m_World->m_Settings->m_SteamAPIKey.empty()) + if (!m_World->m_Settings->GetSteamAPIKey().empty()) { if (auto client = m_World->m_Settings->GetHTTPClient()) { m_TF2PlaytimeFetched = true; m_TF2Playtime = SteamAPI::GetTF2PlaytimeAsync( - m_World->m_Settings->m_SteamAPIKey, GetSteamID(), *client); + m_World->m_Settings->GetSteamAPIKey(), GetSteamID(), *client); } } } @@ -814,13 +814,13 @@ auto WorldState::PlayerSummaryUpdateAction::SendRequest( if (!client) return {}; - if (state->m_Settings->m_SteamAPIKey.empty()) + if (state->m_Settings->GetSteamAPIKey().empty()) return {}; std::vector steamIDs = Take100(collection); return SteamAPI::GetPlayerSummariesAsync( - state->m_Settings->m_SteamAPIKey, std::move(steamIDs), *client); + state->m_Settings->GetSteamAPIKey(), std::move(steamIDs), *client); } void WorldState::PlayerSummaryUpdateAction::OnDataReady(WorldState*& state, @@ -841,12 +841,12 @@ auto WorldState::PlayerBansUpdateAction::SendRequest(state_type& state, if (!client) return {}; - if (state->m_Settings->m_SteamAPIKey.empty()) + if (state->m_Settings->GetSteamAPIKey().empty()) return {}; std::vector steamIDs = Take100(collection); return SteamAPI::GetPlayerBansAsync( - state->m_Settings->m_SteamAPIKey, std::move(steamIDs), *client); + state->m_Settings->GetSteamAPIKey(), std::move(steamIDs), *client); } void WorldState::PlayerBansUpdateAction::OnDataReady(state_type& state,