From 20532135580568478ba7dbc98150604751454e99 Mon Sep 17 00:00:00 2001 From: Arthri <41360489+a@users.noreply.github.com> Date: Sat, 8 Feb 2025 07:13:03 +0000 Subject: [PATCH 1/6] Bounce infinite or NaN velocity / position --- TShockAPI/Bouncer.cs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/TShockAPI/Bouncer.cs b/TShockAPI/Bouncer.cs index 0f5f6e3c..bfcd1836 100644 --- a/TShockAPI/Bouncer.cs +++ b/TShockAPI/Bouncer.cs @@ -504,6 +504,14 @@ namespace TShockAPI return; } + if (!float.IsFinite(pos.X) || !float.IsFinite(pos.Y)) + { + TShock.Log.ConsoleInfo(GetString("Bouncer / OnPlayerUpdate force kicked (attempted to set position to infinity or NaN) from {0}", args.Player.Name)); + args.Player.Kick(GetString("Detected DOOM set to ON position."), true, true); + args.Handled = true; + return; + } + if (pos.X < 0 || pos.Y < 0 || pos.X >= Main.maxTilesX * 16 - 16 || pos.Y >= Main.maxTilesY * 16 - 16) { TShock.Log.ConsoleDebug(GetString("Bouncer / OnPlayerUpdate rejected from (position check) {0}", args.Player.Name)); @@ -1072,6 +1080,22 @@ namespace TShockAPI bool noDelay = args.NoDelay; short type = args.Type; + if (!float.IsFinite(pos.X) || !float.IsFinite(pos.Y)) + { + TShock.Log.ConsoleInfo(GetString("Bouncer / OnItemDrop force kicked (attempted to set position to infinity or NaN) from {0}", args.Player.Name)); + args.Player.Kick(GetString("Detected DOOM set to ON position."), true, true); + args.Handled = true; + return; + } + + if (!float.IsFinite(vel.X) || !float.IsFinite(vel.Y)) + { + TShock.Log.ConsoleInfo(GetString("Bouncer / OnItemDrop force kicked (attempted to set velocity to infinity or NaN) from {0}", args.Player.Name)); + args.Player.Kick(GetString("Detected DOOM set to ON position."), true, true); + args.Handled = true; + return; + } + // player is attempting to crash clients if (type < -48 || type >= Terraria.ID.ItemID.Count) { @@ -1175,6 +1199,22 @@ namespace TShockAPI int index = args.Index; float[] ai = args.Ai; + if (!float.IsFinite(pos.X) || !float.IsFinite(pos.Y)) + { + TShock.Log.ConsoleInfo(GetString("Bouncer / OnNewProjectile force kicked (attempted to set position to infinity or NaN) from {0}", args.Player.Name)); + args.Player.Kick(GetString("Detected DOOM set to ON position."), true, true); + args.Handled = true; + return; + } + + if (!float.IsFinite(vel.X) || !float.IsFinite(vel.Y)) + { + TShock.Log.ConsoleInfo(GetString("Bouncer / OnNewProjectile force kicked (attempted to set velocity to infinity or NaN) from {0}", args.Player.Name)); + args.Player.Kick(GetString("Detected DOOM set to ON position."), true, true); + args.Handled = true; + return; + } + if (index > Main.maxProjectiles) { TShock.Log.ConsoleDebug(GetString("Bouncer / OnNewProjectile rejected from above projectile limit from {0}", args.Player.Name)); From 64d819bebb086f92e55bcd6d1ae26d64d6eb7af0 Mon Sep 17 00:00:00 2001 From: Arthri <41360489+a@users.noreply.github.com> Date: Sat, 8 Feb 2025 07:18:06 +0000 Subject: [PATCH 2/6] Changelog entry --- docs/changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.md b/docs/changelog.md index afcfc727..505419e2 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -95,6 +95,7 @@ Use past tense when adding new entries; sign your name off when you add or chang * * Ensured `TSPlayer.PlayerData` is non-null whilst syncing loadouts. (@drunderscore) * * Detected invalid installations, by checking for a file named `TerrariaServer.exe`. (@drunderscore) * This made the two most common installation mistakes (extracting into the Terraria client directory, and extracting TShock 5 or newer into a TShock 4 or older install) prompt the user with a more useful diagnostic, rather than (likely) crashing moments later. +* Changed Bouncer to block updates which set the following fields to infinity or NaN: player position, projectile position, projectile velocity, item position, and item velocity. (@Arthri) ## TShock 5.2.1 * Updated `TSPlayer.GodMode`. (@AgaSpace) From 1e23785a04dfcedb139f00ed86d0666640844361 Mon Sep 17 00:00:00 2001 From: LaoSparrow Date: Fri, 28 Feb 2025 23:01:40 +0800 Subject: [PATCH 3/6] fix(Bouncer/SendTileRectHandler): MatchRemoval incorrect check range --- TShockAPI/Handlers/SendTileRectHandler.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/TShockAPI/Handlers/SendTileRectHandler.cs b/TShockAPI/Handlers/SendTileRectHandler.cs index d85185b8..e5e3f360 100644 --- a/TShockAPI/Handlers/SendTileRectHandler.cs +++ b/TShockAPI/Handlers/SendTileRectHandler.cs @@ -327,7 +327,7 @@ namespace TShockAPI.Handlers private bool MatchRemoval(TSPlayer player, TileRect rect) { - for (int x = rect.X; x < rect.Y + rect.Width; x++) + for (int x = rect.X; x < rect.X + rect.Width; x++) { for (int y = rect.Y; y < rect.Y + rect.Height; y++) { @@ -910,7 +910,7 @@ namespace TShockAPI.Handlers } } - /* + /* * This is a copy of the `WorldGen.Convert` method with the following precise changes: * - Added a `MockTile tile` parameter * - Changed the `i` and `j` parameters to `k` and `l` @@ -921,7 +921,7 @@ namespace TShockAPI.Handlers * - Removed the ifs checking the bounds of the tile and wall types * - Removed branches that would call `WorldGen.KillTile` * - Changed branches depending on randomness to instead set the property to both values after one another - * + * * This overall leads to a method that can be called on a MockTile and real-world coordinates and will spit out the proper conversion changes into the MockTile. */ From 28aa3aea482badcd54344d910c7f8811a8e1a99e Mon Sep 17 00:00:00 2001 From: LaoSparrow Date: Sat, 1 Mar 2025 01:42:18 +0800 Subject: [PATCH 4/6] fix(Bouncer/SendTileRectHandler): tile rect changes not synced between clients && unable to place HatRack --- TShockAPI/Handlers/SendTileRectHandler.cs | 67 +++++++++++++---------- 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/TShockAPI/Handlers/SendTileRectHandler.cs b/TShockAPI/Handlers/SendTileRectHandler.cs index e5e3f360..d66c3ef6 100644 --- a/TShockAPI/Handlers/SendTileRectHandler.cs +++ b/TShockAPI/Handlers/SendTileRectHandler.cs @@ -78,6 +78,13 @@ namespace TShockAPI.Handlers Removal, } + public enum MatchResult + { + NotMatched, + RejectChanges, + BroadcastChanges, + } + private readonly int Width; private readonly int Height; @@ -179,11 +186,11 @@ namespace TShockAPI.Handlers /// The player the operation originates from. /// The tile rectangle of the operation. /// , if the rect matches this operation and the changes have been applied, otherwise . - public bool Matches(TSPlayer player, TileRect rect) + public MatchResult Matches(TSPlayer player, TileRect rect) { if (rect.Width != Width || rect.Height != Height) { - return false; + return MatchResult.NotMatched; } for (int x = 0; x < rect.Width; x++) @@ -195,7 +202,7 @@ namespace TShockAPI.Handlers { if (tile.Type != TileType) { - return false; + return MatchResult.NotMatched; } } if (Type is MatchType.Placement or MatchType.StateChange) @@ -204,7 +211,7 @@ namespace TShockAPI.Handlers { if (tile.FrameX < 0 || tile.FrameX > MaxFrameX || tile.FrameX % FrameXStep != 0) { - return false; + return MatchResult.NotMatched; } } if (MaxFrameY != IGNORE_FRAME) @@ -214,7 +221,7 @@ namespace TShockAPI.Handlers // this is the only tile type sent in a tile rect where the frame have a different pattern (56, 74, 92 instead of 54, 72, 90) if (!(TileType == TileID.LunarMonolith && tile.FrameY % FrameYStep == 2)) { - return false; + return MatchResult.NotMatched; } } } @@ -223,7 +230,7 @@ namespace TShockAPI.Handlers { if (tile.Active) { - return false; + return MatchResult.NotMatched; } } } @@ -236,7 +243,7 @@ namespace TShockAPI.Handlers if (!player.HasBuildPermission(x, y)) { // for simplicity, let's pretend that the edit was valid, but do not execute it - return true; + return MatchResult.RejectChanges; } } } @@ -257,10 +264,10 @@ namespace TShockAPI.Handlers } } - return false; + return MatchResult.NotMatched; } - private bool MatchPlacement(TSPlayer player, TileRect rect) + private MatchResult MatchPlacement(TSPlayer player, TileRect rect) { for (int x = rect.X; x < rect.Y + rect.Width; x++) { @@ -268,7 +275,7 @@ namespace TShockAPI.Handlers { if (Main.tile[x, y].active()) // the client will kill tiles that auto break before placing the object { - return false; + return MatchResult.NotMatched; } } } @@ -277,7 +284,7 @@ namespace TShockAPI.Handlers if (TShock.TileBans.TileIsBanned((short)TileType, player)) { // for simplicity, let's pretend that the edit was valid, but do not execute it - return true; + return MatchResult.RejectChanges; } for (int x = 0; x < rect.Width; x++) @@ -291,10 +298,10 @@ namespace TShockAPI.Handlers } } - return true; + return MatchResult.BroadcastChanges; } - private bool MatchStateChange(TSPlayer player, TileRect rect) + private MatchResult MatchStateChange(TSPlayer player, TileRect rect) { for (int x = rect.X; x < rect.Y + rect.Width; x++) { @@ -302,7 +309,7 @@ namespace TShockAPI.Handlers { if (!Main.tile[x, y].active() || Main.tile[x, y].type != TileType) { - return false; + return MatchResult.NotMatched; } } } @@ -322,10 +329,10 @@ namespace TShockAPI.Handlers } } - return true; + return MatchResult.BroadcastChanges; } - private bool MatchRemoval(TSPlayer player, TileRect rect) + private MatchResult MatchRemoval(TSPlayer player, TileRect rect) { for (int x = rect.X; x < rect.X + rect.Width; x++) { @@ -333,7 +340,7 @@ namespace TShockAPI.Handlers { if (!Main.tile[x, y].active() || Main.tile[x, y].type != TileType) { - return false; + return MatchResult.NotMatched; } } } @@ -348,7 +355,7 @@ namespace TShockAPI.Handlers } } - return true; + return MatchResult.BroadcastChanges; } } @@ -364,7 +371,7 @@ namespace TShockAPI.Handlers TileRectMatch.Placement(2, 3, TileID.TargetDummy, 54, 36, 18, 18), TileRectMatch.Placement(3, 4, TileID.TeleportationPylon, 468, 54, 18, 18), TileRectMatch.Placement(2, 3, TileID.DisplayDoll, 126, 36, 18, 18), - TileRectMatch.Placement(2, 3, TileID.HatRack, 90, 54, 18, 18), + TileRectMatch.Placement(3, 4, TileID.HatRack, 90, 54, 18, 18), TileRectMatch.Placement(2, 2, TileID.ItemFrame, 162, 18, 18, 18), TileRectMatch.Placement(3, 3, TileID.WeaponsRack2, 90, 36, 18, 18), TileRectMatch.Placement(1, 1, TileID.FoodPlatter, 18, 0, 18, 18), @@ -436,7 +443,7 @@ namespace TShockAPI.Handlers TShock.Log.ConsoleDebug(GetString($"Bouncer / SendTileRect rejected from throttle from {args.Player.Name}")); // send correcting data - args.Player.SendTileRect(args.TileX, args.TileY, args.Length, args.Width); + args.Player.SendTileRect(args.TileX, args.TileY, args.Width, args.Length); return; } @@ -446,7 +453,7 @@ namespace TShockAPI.Handlers TShock.Log.ConsoleDebug(GetString($"Bouncer / SendTileRect rejected from being disabled from {args.Player.Name}")); // send correcting data - args.Player.SendTileRect(args.TileX, args.TileY, args.Length, args.Width); + args.Player.SendTileRect(args.TileX, args.TileY, args.Width, args.Length); return; } @@ -468,7 +475,7 @@ namespace TShockAPI.Handlers TShock.Log.ConsoleDebug(GetString($"Bouncer / SendTileRect reimplemented from {args.Player.Name}")); // send correcting data - args.Player.SendTileRect(args.TileX, args.TileY, args.Length, args.Width); + args.Player.SendTileRect(args.TileX, args.TileY, args.Width, args.Length); return; } @@ -478,7 +485,7 @@ namespace TShockAPI.Handlers TShock.Log.ConsoleDebug(GetString($"Bouncer / SendTileRect rejected from out of range from {args.Player.Name}")); // send correcting data - args.Player.SendTileRect(args.TileX, args.TileY, args.Length, args.Width); + args.Player.SendTileRect(args.TileX, args.TileY, args.Width, args.Length); return; } @@ -488,19 +495,23 @@ namespace TShockAPI.Handlers TShock.Log.ConsoleDebug(GetString($"Bouncer / SendTileRect reimplemented from {args.Player.Name}")); // send correcting data - args.Player.SendTileRect(args.TileX, args.TileY, args.Length, args.Width); + args.Player.SendTileRect(args.TileX, args.TileY, args.Width, args.Length); return; } // check if the rect matches any valid operation foreach (TileRectMatch match in Matches) { - if (match.Matches(args.Player, rect)) + var result = match.Matches(args.Player, rect); + if (result != TileRectMatch.MatchResult.NotMatched) { TShock.Log.ConsoleDebug(GetString($"Bouncer / SendTileRect reimplemented from {args.Player.Name}")); // send correcting data - args.Player.SendTileRect(args.TileX, args.TileY, args.Length, args.Width); + if (result == TileRectMatch.MatchResult.RejectChanges) + args.Player.SendTileRect(args.TileX, args.TileY, args.Width, args.Length); + if (result == TileRectMatch.MatchResult.BroadcastChanges) + TSPlayer.All.SendTileRect(args.TileX, args.TileY, args.Width, args.Length); return; } } @@ -511,14 +522,14 @@ namespace TShockAPI.Handlers TShock.Log.ConsoleDebug(GetString($"Bouncer / SendTileRect reimplemented from {args.Player.Name}")); // send correcting data - args.Player.SendTileRect(args.TileX, args.TileY, args.Length, args.Width); + args.Player.SendTileRect(args.TileX, args.TileY, args.Width, args.Length); return; } TShock.Log.ConsoleDebug(GetString($"Bouncer / SendTileRect rejected from matches from {args.Player.Name}")); // send correcting data - args.Player.SendTileRect(args.TileX, args.TileY, args.Length, args.Width); + args.Player.SendTileRect(args.TileX, args.TileY, args.Width, args.Length); return; } From 61e574ce7230544ce42dc3f730fda99d4843ad75 Mon Sep 17 00:00:00 2001 From: LaoSparrow Date: Sat, 1 Mar 2025 02:02:24 +0800 Subject: [PATCH 5/6] docs(changelog): update entries --- docs/changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.md b/docs/changelog.md index afcfc727..147d5559 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -95,6 +95,10 @@ Use past tense when adding new entries; sign your name off when you add or chang * * Ensured `TSPlayer.PlayerData` is non-null whilst syncing loadouts. (@drunderscore) * * Detected invalid installations, by checking for a file named `TerrariaServer.exe`. (@drunderscore) * This made the two most common installation mistakes (extracting into the Terraria client directory, and extracting TShock 5 or newer into a TShock 4 or older install) prompt the user with a more useful diagnostic, rather than (likely) crashing moments later. +* Updated `TShockAPI.Handlers.SendTileRectHandler` (@LaoSparrow): + * Fixed incorrect validating range in `TileRectMatch.MatchRemoval`. + * Fixed tile rect changes (e.g. turning on and off campfires) are not synced between clients. + * Fixed unable to place Hat Rack without permission `tshock.ignore.sendtilesquare`. ## TShock 5.2.1 * Updated `TSPlayer.GodMode`. (@AgaSpace) From 740c5c9250c262e2602292ac9e5b2d396ec0f842 Mon Sep 17 00:00:00 2001 From: LaoSparrow Date: Sat, 1 Mar 2025 03:41:30 +0800 Subject: [PATCH 6/6] fix(GetDataHandlers): handle and ignore `NpcItemStrike(msgid 24)` --- TShockAPI/GetDataHandlers.cs | 8 ++++++++ docs/changelog.md | 1 + 2 files changed, 9 insertions(+) diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index 8f70fd2c..22cfadb3 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -95,6 +95,7 @@ namespace TShockAPI { PacketTypes.TileSendSquare, HandleSendTileRect }, { PacketTypes.ItemDrop, HandleItemDrop }, { PacketTypes.ItemOwner, HandleItemOwner }, + { PacketTypes.NpcItemStrike, HandleNpcItemStrike }, { PacketTypes.ProjectileNew, HandleProjectileNew }, { PacketTypes.NpcStrike, HandleNpcStrike }, { PacketTypes.ProjectileDestroy, HandleProjectileKill }, @@ -2945,6 +2946,13 @@ namespace TShockAPI return false; } + private static bool HandleNpcItemStrike(GetDataHandlerArgs args) + { + // Never sent by vanilla client, ignore this + TShock.Log.ConsoleDebug(GetString("GetDataHandlers / HandleNpcItemStrike surprise packet! Someone tell the TShock team! {0}", args.Player.Name)); + return true; + } + private static bool HandleProjectileNew(GetDataHandlerArgs args) { short ident = args.Data.ReadInt16(); diff --git a/docs/changelog.md b/docs/changelog.md index 147d5559..cf4e43f2 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -99,6 +99,7 @@ Use past tense when adding new entries; sign your name off when you add or chang * Fixed incorrect validating range in `TileRectMatch.MatchRemoval`. * Fixed tile rect changes (e.g. turning on and off campfires) are not synced between clients. * Fixed unable to place Hat Rack without permission `tshock.ignore.sendtilesquare`. +* Updated `GetDataHandlers` to ignore `NpcItemStrike(msgid 24)`, which should never be sent by a vanilla client. (@LaoSparrow) ## TShock 5.2.1 * Updated `TSPlayer.GodMode`. (@AgaSpace)