From ce5ee0d6230fa96954d317bf9ef3f74e5d746525 Mon Sep 17 00:00:00 2001 From: Patrikkk Date: Mon, 1 Jun 2020 17:02:27 +0200 Subject: [PATCH 1/3] Add HandleSyncCavernMonsterType This packet is never sent from the client to the server in a normal scenario. Although with modded clients, a packet can be sent to modify the cavernMonsterType of the server world and have the world spawn defined NPC types. Can be used to have the server randomly spawn bosses on players in caverns. Is it okay to have this simple handling in GetDataHandlers? A seperate class felt like an overkill. Moved the HandleSyncRevengeMarker packet handler to it's "correct" position, so I won't have merge issues between my last PR. As I've mentioned there, we have the Packets in their numerical order. --- CHANGELOG.md | 2 +- TShockAPI/GetDataHandlers.cs | 33 ++++++++++++++++++++------------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 121cb25a..49ffe302 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when adding new entries; sign your name off when you add or change something. This should primarily be things like user changes, not necessarily codebase changes unless it's really relevant or large. ## Upcoming Changes -* Your change goes here! +* Handling SyncCavernMonsterType packet to prevent an exploit where players could modify the server's cavern monster types and make the server spawn any NPCs - including bosses - onto other players. ## TShock 4.4.0 (Pre-release 10) * Fix all rope coils. (@Olink) diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index b5c9bbb0..f2083f90 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -151,9 +151,10 @@ namespace TShockAPI { PacketTypes.CrystalInvasionStart, HandleOldOnesArmy }, { PacketTypes.PlayerHurtV2, HandlePlayerDamageV2 }, { PacketTypes.PlayerDeathV2, HandlePlayerKillMeV2 }, + { PacketTypes.SyncRevengeMarker, HandleSyncRevengeMarker }, { PacketTypes.FishOutNPC, HandleFishOutNPC }, { PacketTypes.FoodPlatterTryPlacing, HandleFoodPlatterTryPlacing }, - { PacketTypes.SyncRevengeMarker, HandleSyncRevengeMarker } + { PacketTypes.SyncCavernMonsterType, HandleSyncCavernMonsterType } }; } @@ -3671,6 +3672,21 @@ namespace TShockAPI return false; } + private static bool HandleSyncRevengeMarker(GetDataHandlerArgs args) + { + int uniqueID = args.Data.ReadInt32(); + Vector2 location = args.Data.ReadVector2(); + int netId = args.Data.ReadInt32(); + float npcHpPercent = args.Data.ReadSingle(); + int npcTypeAgainstDiscouragement = args.Data.ReadInt32(); //tfw the argument is Type Against Discouragement + int npcAiStyleAgainstDiscouragement = args.Data.ReadInt32(); //see ^ + int coinsValue = args.Data.ReadInt32(); + float baseValue = args.Data.ReadSingle(); + bool spawnedFromStatus = args.Data.ReadBoolean(); + + return false; + } + private static bool HandleFishOutNPC(GetDataHandlerArgs args) { ushort tileX = args.Data.ReadUInt16(); @@ -3697,19 +3713,10 @@ namespace TShockAPI return false; } - private static bool HandleSyncRevengeMarker(GetDataHandlerArgs args) + private static bool HandleSyncCavernMonsterType(GetDataHandlerArgs args) { - int uniqueID = args.Data.ReadInt32(); - Vector2 location = args.Data.ReadVector2(); - int netId = args.Data.ReadInt32(); - float npcHpPercent = args.Data.ReadSingle(); - int npcTypeAgainstDiscouragement = args.Data.ReadInt32(); //tfw the argument is Type Against Discouragement - int npcAiStyleAgainstDiscouragement = args.Data.ReadInt32(); //see ^ - int coinsValue = args.Data.ReadInt32(); - float baseValue = args.Data.ReadSingle(); - bool spawnedFromStatus = args.Data.ReadBoolean(); - - return false; + TShock.Log.ConsoleDebug($"HandleSyncCavernMonsterType: Player is trying to modify NPC cavernMonsterType; this is a crafted packet! - From {args.Player.Name}"); + return true; } public enum EditAction From cd58d79322aa018e90da735bebe465b8ead23141 Mon Sep 17 00:00:00 2001 From: Patrikkk Date: Tue, 2 Jun 2020 11:34:10 +0200 Subject: [PATCH 2/3] Kick player on attempting cavern npc type modification. --- TShockAPI/GetDataHandlers.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index f2083f90..f86104d9 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -3715,6 +3715,7 @@ namespace TShockAPI private static bool HandleSyncCavernMonsterType(GetDataHandlerArgs args) { + args.Player.Kick("Exploit attempt detected!"); TShock.Log.ConsoleDebug($"HandleSyncCavernMonsterType: Player is trying to modify NPC cavernMonsterType; this is a crafted packet! - From {args.Player.Name}"); return true; } From 0619092ba5e9a140e136c451e7c7ed5c654db40f Mon Sep 17 00:00:00 2001 From: Patrikkk Date: Tue, 2 Jun 2020 11:41:41 +0200 Subject: [PATCH 3/3] Fix identing after conflict resolve merge. --- TShockAPI/GetDataHandlers.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index 00172763..a73c7aef 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -152,7 +152,7 @@ namespace TShockAPI { PacketTypes.PlayerHurtV2, HandlePlayerDamageV2 }, { PacketTypes.PlayerDeathV2, HandlePlayerKillMeV2 }, { PacketTypes.Emoji, HandleEmoji }, - { PacketTypes.SyncRevengeMarker, HandleSyncRevengeMarker }, + { PacketTypes.SyncRevengeMarker, HandleSyncRevengeMarker }, { PacketTypes.FishOutNPC, HandleFishOutNPC }, { PacketTypes.FoodPlatterTryPlacing, HandleFoodPlatterTryPlacing }, { PacketTypes.SyncCavernMonsterType, HandleSyncCavernMonsterType } @@ -3603,7 +3603,7 @@ namespace TShockAPI return false; } - private static bool HandleSyncRevengeMarker(GetDataHandlerArgs args) + private static bool HandleSyncRevengeMarker(GetDataHandlerArgs args) { int uniqueID = args.Data.ReadInt32(); Vector2 location = args.Data.ReadVector2(); @@ -3615,7 +3615,7 @@ namespace TShockAPI float baseValue = args.Data.ReadSingle(); bool spawnedFromStatus = args.Data.ReadBoolean(); - return false; + return false; } private static bool HandleFishOutNPC(GetDataHandlerArgs args)