From 2177d75066bd47c200547126d27b3c54fdb81c62 Mon Sep 17 00:00:00 2001 From: Patrikkk Date: Mon, 1 Jun 2020 18:31:30 +0200 Subject: [PATCH 1/5] Add SyncTilePicking event. --- CHANGELOG.md | 2 +- TShockAPI/GetDataHandlers.cs | 93 +++++++++++++++++++++++++++++------- 2 files changed, 77 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 121cb25a..8dc088af 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! +* Add SyncTilePicking event. This is called when a player damages a tile. ## TShock 4.4.0 (Pre-release 10) * Fix all rope coils. (@Olink) diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index b5c9bbb0..64f57b9c 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.SyncTilePicking, HandleSyncTilePicking }, + { PacketTypes.SyncRevengeMarker, HandleSyncRevengeMarker }, { PacketTypes.FishOutNPC, HandleFishOutNPC }, - { PacketTypes.FoodPlatterTryPlacing, HandleFoodPlatterTryPlacing }, - { PacketTypes.SyncRevengeMarker, HandleSyncRevengeMarker } + { PacketTypes.FoodPlatterTryPlacing, HandleFoodPlatterTryPlacing } }; } @@ -1866,6 +1867,51 @@ namespace TShockAPI return args.Handled; } + /// + /// For use in a SyncTilePicking event. + /// + public class SyncTilePickingEventArgs : GetDataHandledEventArgs + { + /// + /// The player index in the packet, who sends the tile picking data. + /// + public byte PlayerIndex { get; set; } + /// + /// The X world position of the tile that is being picked. + /// + public short TileX { get; set; } + /// + /// The Y world position of the tile that is being picked. + /// + public short TileY { get; set; } + /// + /// The damage that is being dealt on the tile. + /// + public byte TileDamage { get; set; } + } + /// + /// Called when a player hits and damages a tile. + /// + public static HandlerList SyncTilePicking = new HandlerList(); + private static bool OnSyncTilePicking(TSPlayer player, MemoryStream data, byte playerIndex, short tileX, short tileY, byte tileDamage) + { + if (SyncTilePicking == null) + return false; + + var args = new SyncTilePickingEventArgs + { + Player = player, + Data = data, + PlayerIndex = playerIndex, + TileX = tileX, + TileY = tileY, + TileDamage = tileDamage + }; + SyncTilePicking.Invoke(null, args); + return args.Handled; + } + + /// /// For use in a FishOutNPC event. /// @@ -3671,6 +3717,34 @@ namespace TShockAPI return false; } + private static bool HandleSyncTilePicking(GetDataHandlerArgs args) + { + byte playerIndex = args.Data.ReadInt8(); + short tileX = args.Data.ReadInt16(); + short tileY = args.Data.ReadInt16(); + byte damage = args.Data.ReadInt8(); + + if (OnSyncTilePicking(args.Player, args.Data, playerIndex, tileX, tileY, damage)) + return true; + + 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,21 +3771,6 @@ 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; - } - public enum EditAction { KillTile = 0, From cffdadbc802fdb5219c949f9e6eaf858d5dc9dde Mon Sep 17 00:00:00 2001 From: Patrikkk Date: Tue, 2 Jun 2020 19:10:34 +0200 Subject: [PATCH 2/5] Fix previous conflict resolve merging. --- TShockAPI/GetDataHandlers.cs | 43 ++++++++++++------------------------ 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index 0ae658fd..496a7053 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -3696,35 +3696,6 @@ 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 HandleLandGolfBallInCup(GetDataHandlerArgs args) - { - byte playerIndex = args.Data.ReadInt8(); - ushort tileX = args.Data.ReadUInt16(); - ushort tileY = args.Data.ReadUInt16(); - ushort hits = args.Data.ReadUInt16(); - ushort projectileType = args.Data.ReadUInt16(); - - if (OnLandGolfBallInCup(args.Player, args.Data, playerIndex, tileX, tileY, hits, projectileType)) - return true; - - return false; - } private static bool HandleSyncTilePicking(GetDataHandlerArgs args) { @@ -3750,6 +3721,20 @@ namespace TShockAPI int coinsValue = args.Data.ReadInt32(); float baseValue = args.Data.ReadSingle(); bool spawnedFromStatus = args.Data.ReadBoolean(); + + return false; + } + + private static bool HandleLandGolfBallInCup(GetDataHandlerArgs args) + { + byte playerIndex = args.Data.ReadInt8(); + ushort tileX = args.Data.ReadUInt16(); + ushort tileY = args.Data.ReadUInt16(); + ushort hits = args.Data.ReadUInt16(); + ushort projectileType = args.Data.ReadUInt16(); + + if (OnLandGolfBallInCup(args.Player, args.Data, playerIndex, tileX, tileY, hits, projectileType)) + return true; return false; } From e738d8e79442f1aece17b00fa28fb81128325e31 Mon Sep 17 00:00:00 2001 From: Patrikkk Date: Tue, 2 Jun 2020 19:20:27 +0200 Subject: [PATCH 3/5] Implementing SyncTilePickingHandler. Patching tile damage related exploits. With this packet, players could kick all players by sending invalid world position data. --- CHANGELOG.md | 2 +- TShockAPI/Bouncer.cs | 4 +++ TShockAPI/Handlers/SyncTilePickingHandler.cs | 36 ++++++++++++++++++++ TShockAPI/TShockAPI.csproj | 1 + 4 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 TShockAPI/Handlers/SyncTilePickingHandler.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 58a50a10..a3ce8308 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin * Adding EmojiHandler to handle an exploit. Adding `tshock.sendemoji` permission and checks. Added this permission to guest group by default. * 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. * Added LandGolfBallInCup event which is accessible for developers to work with, as well as LandGolfBallInCup handler to handle exploits where players could send direct packets to trigger and imitate golf ball cup landing anywhere in the game world. Added two public lists in Handlers.LandGolfBallInCupHandler: GolfBallProjectileIDs and GolfClubItemIDs. (@Patrikkk) -* Add SyncTilePicking event. This is called when a player damages a tile. +* Add SyncTilePicking event. This is called when a player damages a tile. Implementing SyncTilePickingHandler and patching tile damaging related exploits. (Prevent player sending invalid world position data to kick players. Prevent player ID spoofing) ## TShock 4.4.0 (Pre-release 10) * Fix all rope coils. (@Olink) diff --git a/TShockAPI/Bouncer.cs b/TShockAPI/Bouncer.cs index 8029031b..6f2a2ae4 100644 --- a/TShockAPI/Bouncer.cs +++ b/TShockAPI/Bouncer.cs @@ -40,6 +40,7 @@ namespace TShockAPI internal Handlers.NetModules.NetModulePacketHandler NetModuleHandler { get; set; } internal Handlers.EmojiHandler EmojiHandler { get; set; } internal Handlers.LandGolfBallInCupHandler LandGolfBallInCupHandler { get; set; } + internal Handlers.SyncTilePickingHandler SyncTilePickingHandler { get; set; } /// Constructor call initializes Bouncer and related functionality. /// A new Bouncer. @@ -57,6 +58,9 @@ namespace TShockAPI LandGolfBallInCupHandler = new Handlers.LandGolfBallInCupHandler(); GetDataHandlers.LandGolfBallInCup += LandGolfBallInCupHandler.OnReceive; + SyncTilePickingHandler = new Handlers.SyncTilePickingHandler(); + GetDataHandlers.SyncTilePicking += SyncTilePickingHandler.OnReceive; + // Setup hooks GetDataHandlers.GetSection += OnGetSection; GetDataHandlers.PlayerUpdate += OnPlayerUpdate; diff --git a/TShockAPI/Handlers/SyncTilePickingHandler.cs b/TShockAPI/Handlers/SyncTilePickingHandler.cs new file mode 100644 index 00000000..a03bec66 --- /dev/null +++ b/TShockAPI/Handlers/SyncTilePickingHandler.cs @@ -0,0 +1,36 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Terraria; +using static TShockAPI.GetDataHandlers; + +namespace TShockAPI.Handlers +{ + class SyncTilePickingHandler : IPacketHandler + { + /// + /// Invoked when player damages a tile. + /// + /// + /// + public void OnReceive(object sender, SyncTilePickingEventArgs args) + { + if (args.PlayerIndex != args.Player.Index) + { + TShock.Log.ConsoleDebug($"SyncTilePickingHandler: SyncTilePicking packet rejected for ID spoofing. Expected {args.Player.Index}, received {args.PlayerIndex} from {args.Player.Name}."); + args.Handled = true; + return; + } + + if (args.TileX > Main.maxTilesX || args.TileX < 0 + || args.TileY > Main.maxTilesY || args.TileY < 0) + { + TShock.Log.ConsoleDebug($"SyncTilePickingHandler: X and Y position is out of world bounds! - From {args.Player.Name}"); + args.Handled = true; + return; + } + } + } +} diff --git a/TShockAPI/TShockAPI.csproj b/TShockAPI/TShockAPI.csproj index 495127c8..f9885393 100644 --- a/TShockAPI/TShockAPI.csproj +++ b/TShockAPI/TShockAPI.csproj @@ -100,6 +100,7 @@ + From 7487afafb660be8fb908599a2da383af3590a91c Mon Sep 17 00:00:00 2001 From: Patrikkk Date: Tue, 2 Jun 2020 19:26:25 +0200 Subject: [PATCH 4/5] SyncTilePickingHandler - Remove redundant check. Properly document method. --- TShockAPI/Handlers/SyncTilePickingHandler.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/TShockAPI/Handlers/SyncTilePickingHandler.cs b/TShockAPI/Handlers/SyncTilePickingHandler.cs index a03bec66..d7ad1e13 100644 --- a/TShockAPI/Handlers/SyncTilePickingHandler.cs +++ b/TShockAPI/Handlers/SyncTilePickingHandler.cs @@ -11,19 +11,12 @@ namespace TShockAPI.Handlers class SyncTilePickingHandler : IPacketHandler { /// - /// Invoked when player damages a tile. + /// Invoked when player damages a tile. Rejects the packet if its out of world bounds. /// /// /// public void OnReceive(object sender, SyncTilePickingEventArgs args) { - if (args.PlayerIndex != args.Player.Index) - { - TShock.Log.ConsoleDebug($"SyncTilePickingHandler: SyncTilePicking packet rejected for ID spoofing. Expected {args.Player.Index}, received {args.PlayerIndex} from {args.Player.Name}."); - args.Handled = true; - return; - } - if (args.TileX > Main.maxTilesX || args.TileX < 0 || args.TileY > Main.maxTilesY || args.TileY < 0) { From 4587ac4d5cfe9fd035970810b12ec471253e2229 Mon Sep 17 00:00:00 2001 From: Patrikkk Date: Tue, 2 Jun 2020 19:27:42 +0200 Subject: [PATCH 5/5] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3ce8308..823bb7de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin * Adding EmojiHandler to handle an exploit. Adding `tshock.sendemoji` permission and checks. Added this permission to guest group by default. * 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. * Added LandGolfBallInCup event which is accessible for developers to work with, as well as LandGolfBallInCup handler to handle exploits where players could send direct packets to trigger and imitate golf ball cup landing anywhere in the game world. Added two public lists in Handlers.LandGolfBallInCupHandler: GolfBallProjectileIDs and GolfClubItemIDs. (@Patrikkk) -* Add SyncTilePicking event. This is called when a player damages a tile. Implementing SyncTilePickingHandler and patching tile damaging related exploits. (Prevent player sending invalid world position data to kick players. Prevent player ID spoofing) +* Add SyncTilePicking event. This is called when a player damages a tile. Implementing SyncTilePickingHandler and patching tile damaging related exploits. (Preventing player sending invalid world position data which disconnects other players.) ## TShock 4.4.0 (Pre-release 10) * Fix all rope coils. (@Olink)