From 7be312c4c6eb8cbaaf716776b54c835dd3e309a4 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 1 Mar 2019 09:25:16 +0100 Subject: [PATCH 1/4] Implement rate limiting times for game actions. --- src/openrct2/actions/GameAction.h | 9 +++ src/openrct2/network/Network.cpp | 89 ++++++++++++++++------------ src/openrct2/network/NetworkPlayer.h | 3 +- 3 files changed, 61 insertions(+), 40 deletions(-) diff --git a/src/openrct2/actions/GameAction.h b/src/openrct2/actions/GameAction.h index 2210d3fdff..89cc8aa0db 100644 --- a/src/openrct2/actions/GameAction.h +++ b/src/openrct2/actions/GameAction.h @@ -188,6 +188,15 @@ public: return const_cast(*this).Serialise(stream); } + /** + * Override this to specify the wait time in milliseconds the player is required to wait before + * being able to execute it again. + */ + virtual uint32_t GetCooldownTime() const + { + return 0; + } + /** * Query the result of the game action without changing the game state. */ diff --git a/src/openrct2/network/Network.cpp b/src/openrct2/network/Network.cpp index 7253472937..7f5f7d00ca 100644 --- a/src/openrct2/network/Network.cpp +++ b/src/openrct2/network/Network.cpp @@ -203,6 +203,7 @@ public: std::string ServerProviderWebsite; private: + void DecayCooldown(NetworkPlayer* player); void CloseConnection(); bool ProcessConnection(NetworkConnection& connection); @@ -314,6 +315,8 @@ private: uint8_t default_group = 0; uint32_t _commandId; uint32_t _actionId; + uint32_t _lastUpdateTime = 0; + uint32_t _currentDeltaTime = 0; std::string _chatLogPath; std::string _chatLogFilenameFormat = "%Y%m%d-%H%M%S.txt"; std::string _serverLogPath; @@ -451,6 +454,21 @@ void Network::Close() } } +void Network::DecayCooldown(NetworkPlayer* player) +{ + if (player == nullptr) + return; // No valid connection yet. + + for (auto it = std::begin(player->CooldownTime); it != std::end(player->CooldownTime);) + { + it->second -= _currentDeltaTime; + if (it->second <= 0) + it = player->CooldownTime.erase(it); + else + it++; + } +} + void Network::CloseConnection() { if (mode == NETWORK_MODE_CLIENT) @@ -671,6 +689,11 @@ void Network::Update() { _closeLock = true; + // Update is not necessarily called per game tick, maintain our own delta time + uint32_t ticks = platform_get_ticks(); + _currentDeltaTime = std::max(ticks - _lastUpdateTime, 1); + _lastUpdateTime = ticks; + switch (GetMode()) { case NETWORK_MODE_SERVER: @@ -716,6 +739,7 @@ void Network::UpdateServer() } else { + DecayCooldown((*it)->Player); it++; } } @@ -2737,58 +2761,29 @@ void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa uint32_t tick; uint32_t actionType; - if (!connection.Player) + NetworkPlayer* player = connection.Player; + if (player == nullptr) { return; } packet >> tick >> actionType; - // tick count is different by time last_action_time is set, keep same value + // Don't let clients send pause or quit + if (actionType == GAME_COMMAND_TOGGLE_PAUSE || actionType == GAME_COMMAND_LOAD_OR_QUIT) + { + return; + } + // Check if player's group permission allows command to run - uint32_t ticks = platform_get_ticks(); NetworkGroup* group = GetGroupByID(connection.Player->Group); - if (!group || !group->CanPerformCommand(actionType)) + if (group == nullptr || group->CanPerformCommand(actionType) == false) { Server_Send_SHOWERROR(connection, STR_CANT_DO_THIS, STR_PERMISSION_DENIED); return; } - // In case someone modifies the code / memory to enable cluster build, - // require a small delay in between placing scenery to provide some security, as - // cluster mode is a for loop that runs the place_scenery code multiple times. - if (actionType == GAME_COMMAND_PLACE_SCENERY) - { - if (ticks - connection.Player->LastPlaceSceneryTime < ACTION_COOLDOWN_TIME_PLACE_SCENERY && - // Incase platform_get_ticks() wraps after ~49 days, ignore larger logged times. - ticks > connection.Player->LastPlaceSceneryTime) - { - if (!(group->CanPerformCommand(MISC_COMMAND_TOGGLE_SCENERY_CLUSTER))) - { - Server_Send_SHOWERROR(connection, STR_CANT_DO_THIS, STR_NETWORK_ACTION_RATE_LIMIT_MESSAGE); - return; - } - } - } - // This is to prevent abuse of demolishing rides. Anyone that is not the server - // host will have to wait a small amount of time in between deleting rides. - else if (actionType == GAME_COMMAND_DEMOLISH_RIDE) - { - if (ticks - connection.Player->LastDemolishRideTime < ACTION_COOLDOWN_TIME_DEMOLISH_RIDE && - // Incase platform_get_ticks()() wraps after ~49 days, ignore larger logged times. - ticks > connection.Player->LastDemolishRideTime) - { - Server_Send_SHOWERROR(connection, STR_CANT_DO_THIS, STR_NETWORK_ACTION_RATE_LIMIT_MESSAGE); - return; - } - } - // Don't let clients send pause or quit - else if (actionType == GAME_COMMAND_TOGGLE_PAUSE || actionType == GAME_COMMAND_LOAD_OR_QUIT) - { - return; - } - - // Run game command, and if it is successful send to clients + // Create and enqueue the action. GameAction::Ptr ga = GameActions::Create(actionType); if (ga == nullptr) { @@ -2798,6 +2793,22 @@ void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa return; } + auto cooldownIt = player->CooldownTime.find(actionType); + if (cooldownIt != std::end(player->CooldownTime)) + { + if (cooldownIt->second > 0) + { + Server_Send_SHOWERROR(connection, STR_CANT_DO_THIS, STR_NETWORK_ACTION_RATE_LIMIT_MESSAGE); + return; + } + } + + uint32_t cooldownTime = ga->GetCooldownTime(); + if (cooldownTime > 0) + { + player->CooldownTime[actionType] = cooldownTime; + } + DataSerialiser stream(false); size_t size = packet.Size - packet.BytesRead; stream.GetStream().WriteArray(packet.Read(size), size); diff --git a/src/openrct2/network/NetworkPlayer.h b/src/openrct2/network/NetworkPlayer.h index cf762ad76e..6b128258ac 100644 --- a/src/openrct2/network/NetworkPlayer.h +++ b/src/openrct2/network/NetworkPlayer.h @@ -15,6 +15,7 @@ #include "../world/Sprite.h" #include +#include class NetworkPacket; @@ -36,7 +37,7 @@ public: std::string KeyHash; uint32_t LastDemolishRideTime = 0; uint32_t LastPlaceSceneryTime = 0; - + std::unordered_map CooldownTime; NetworkPlayer() = default; void SetName(const std::string& name); From cfe2fb939e34b1b25c6f9158116f8dacc8d55b1c Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 1 Mar 2019 10:13:39 +0100 Subject: [PATCH 2/4] Add cooldown time to RideDemolishAction --- src/openrct2/actions/RideDemolishAction.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/openrct2/actions/RideDemolishAction.hpp b/src/openrct2/actions/RideDemolishAction.hpp index e74fef9162..28f67d1876 100644 --- a/src/openrct2/actions/RideDemolishAction.hpp +++ b/src/openrct2/actions/RideDemolishAction.hpp @@ -45,6 +45,11 @@ public: { } + uint32_t GetCooldownTime() const override + { + return 1000; + } + void Serialise(DataSerialiser & stream) override { GameAction::Serialise(stream); From 097ed015d867bcc28e8889732b32ebc13b7d45b4 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 1 Mar 2019 10:13:50 +0100 Subject: [PATCH 3/4] Add cooldown time for SmallSceneryPlaceAction --- src/openrct2/actions/SmallSceneryPlaceAction.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/openrct2/actions/SmallSceneryPlaceAction.hpp b/src/openrct2/actions/SmallSceneryPlaceAction.hpp index 7cc9052de1..f20db46d2d 100644 --- a/src/openrct2/actions/SmallSceneryPlaceAction.hpp +++ b/src/openrct2/actions/SmallSceneryPlaceAction.hpp @@ -49,6 +49,11 @@ public: { } + uint32_t GetCooldownTime() const override + { + return 20; + } + uint16_t GetActionFlags() const override { return GameAction::GetActionFlags(); From c0cd1aaef3c1eb679875ddafbcb75026487cc5db Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 1 Mar 2019 11:01:00 +0100 Subject: [PATCH 4/4] Exclude host from rate limiting. --- src/openrct2/network/Network.cpp | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/openrct2/network/Network.cpp b/src/openrct2/network/Network.cpp index 7f5f7d00ca..f22ba345d1 100644 --- a/src/openrct2/network/Network.cpp +++ b/src/openrct2/network/Network.cpp @@ -2793,20 +2793,24 @@ void Network::Server_Handle_GAME_ACTION(NetworkConnection& connection, NetworkPa return; } - auto cooldownIt = player->CooldownTime.find(actionType); - if (cooldownIt != std::end(player->CooldownTime)) + // Player who is hosting is not affected by cooldowns. + if ((player->Flags & NETWORK_PLAYER_FLAG_ISSERVER) == 0) { - if (cooldownIt->second > 0) + auto cooldownIt = player->CooldownTime.find(actionType); + if (cooldownIt != std::end(player->CooldownTime)) { - Server_Send_SHOWERROR(connection, STR_CANT_DO_THIS, STR_NETWORK_ACTION_RATE_LIMIT_MESSAGE); - return; + if (cooldownIt->second > 0) + { + Server_Send_SHOWERROR(connection, STR_CANT_DO_THIS, STR_NETWORK_ACTION_RATE_LIMIT_MESSAGE); + return; + } } - } - uint32_t cooldownTime = ga->GetCooldownTime(); - if (cooldownTime > 0) - { - player->CooldownTime[actionType] = cooldownTime; + uint32_t cooldownTime = ga->GetCooldownTime(); + if (cooldownTime > 0) + { + player->CooldownTime[actionType] = cooldownTime; + } } DataSerialiser stream(false);