diff --git a/CHANGELOG.md b/CHANGELOG.md index 318e82b90..3252c26ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin * Updated server startup language to be more clear when encountering a fatal startup error. Now, the server gives more context as to what happened so that there's a better chance of people being able to help themselves. (@hakusaro) * Added `-worldevil ` command line argument (@NotGeri) * Added PlayerHasBuildPermission hook to PlayerHooks. (@AnzhelikaO, @Killia0) +* Fixed an exploit in which the Ice Block deletion allowance from the Ice Rod bypassed region protection, allowing for deleting all tiles in a protected region and/or replacing them with Ice Blocks. (@punchready) +* Changed SendTileRect handling from a denylist to an allowlist with stricter checks. This prevents essentially all exploits involving this packet. Most notably this stops people from placing arbitrary tiles with arbitrary framing values, which are the root of most exploits. (@punchready) +* Removed the config options `TileRectangleSizeThreshold` and `KickOnTileRectangleSizeThresholdBroken` because they are made obsolete by the new system, which will only allow valid rectangle sizes (at a maximum of only 4 by 4 tiles in 1.4.3.6). (@punchready) ## TShock 4.5.17 * Fixed duplicate characters (twins) after repeatedly logging in as the same character due to connection not being immediately closed during `NetHooks_NameCollision`. (@gohjoseph) diff --git a/TShockAPI/Bouncer.cs b/TShockAPI/Bouncer.cs index dae664610..0f578998a 100644 --- a/TShockAPI/Bouncer.cs +++ b/TShockAPI/Bouncer.cs @@ -401,6 +401,18 @@ internal void OnTileEdit(object sender, GetDataHandlers.TileEditEventArgs args) return; } + // I do not understand the ice tile check enough to be able to modify it, however I do know that it can be used to completely bypass region protection + // This check ensures that build permission is always checked no matter what + if (!args.Player.HasBuildPermission(tileX, tileY)) + { + TShock.Log.ConsoleDebug("Bouncer / OnTileEdit rejected from build from {0} {1} {2}", args.Player.Name, action, editData); + + GetRollbackRectSize(tileX, tileY, out byte width, out byte length, out int offsetY); + args.Player.SendTileRect((short)(tileX - width), (short)(tileY + offsetY), (byte)(width * 2), (byte)(length + 1)); + args.Handled = true; + return; + } + if (editData < 0 || ((action == EditAction.PlaceTile || action == EditAction.ReplaceTile) && editData >= Main.maxTileSets) || ((action == EditAction.PlaceWall || action == EditAction.ReplaceWall) && editData >= Main.maxWallTypes)) diff --git a/TShockAPI/Configuration/TShockConfig.cs b/TShockAPI/Configuration/TShockConfig.cs index d87dfa227..8a5e3321c 100644 --- a/TShockAPI/Configuration/TShockConfig.cs +++ b/TShockAPI/Configuration/TShockConfig.cs @@ -445,14 +445,6 @@ public class TShockSettings [Description("Whether or not to kick users when they surpass the HealOther threshold.")] public bool KickOnHealOtherThresholdBroken = false; - /// Disables a player if this number of tiles is present in a Tile Rectangle packet - [Description("Disables a player if this number of tiles is present in a Tile Rectangle packet")] - public int TileRectangleSizeThreshold = 50; - - /// Whether or not to kick users when they surpass the TileRectangleSize threshold. - [Description("Whether or not to kick users when they surpass the TileRectangleSize threshold.")] - public bool KickOnTileRectangleSizeThresholdBroken = false; - /// Whether or not the server should suppress build permission failure warnings from regions, spawn point, or server edit failure. [Description("Whether or not the server should suppress build permission failure warnings from regions, spawn point, or server edit failure.")] public bool SuppressPermissionFailureNotices = false; diff --git a/TShockAPI/Handlers/SendTileRectHandler.cs b/TShockAPI/Handlers/SendTileRectHandler.cs index b5c0bafd5..dbe80ec63 100644 --- a/TShockAPI/Handlers/SendTileRectHandler.cs +++ b/TShockAPI/Handlers/SendTileRectHandler.cs @@ -20,13 +20,55 @@ namespace TShockAPI.Handlers public class SendTileRectHandler : IPacketHandler { /// - /// Maps grass-type blocks to flowers that can be grown on them with flower boots + /// Maps plant tile types to their valid grass ground tiles when using flower boots /// - public static Dictionary> GrassToPlantMap = new Dictionary> + private static readonly Dictionary> FlowerBootPlantToGrassMap = new Dictionary> { - { TileID.Grass, new List { TileID.Plants, TileID.Plants2 } }, - { TileID.HallowedGrass, new List { TileID.HallowedPlants, TileID.HallowedPlants2 } }, - { TileID.JungleGrass, new List { TileID.JunglePlants, TileID.JunglePlants2 } } + { TileID.Plants, new HashSet() + { + TileID.Grass, TileID.GolfGrass + } }, + { TileID.HallowedPlants, new HashSet() + { + TileID.HallowedGrass, TileID.GolfGrassHallowed + } }, + { TileID.HallowedPlants2, new HashSet() + { + TileID.HallowedGrass, TileID.GolfGrassHallowed + } }, + { TileID.JunglePlants2, new HashSet() + { + TileID.JungleGrass + } }, + }; + + /// + /// Maps plant tile types to a list of valid styles, which are used to determine the FrameX value of the plant tile + /// See `Player.DoBootsEffect_PlaceFlowersOnTile` + /// + private static readonly Dictionary> FlowerBootPlantToStyleMap = new Dictionary>() + { + { TileID.Plants, new HashSet() + { + // The upper line is from a `NextFromList` call + // The lower line is from an additional switch which will add the listed options by adding a random value to a select set of styles + 6, 7, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 24, 27, 30, 33, 36, 39, 42, + 22, 23, 25, 26, 28, 29, 31, 32, 34, 35, 37, 38, 40, 41, 43, 44, + } }, + { TileID.HallowedPlants, new HashSet() + { + // 5 is intentionally missing here because it is being skipped by vanilla + 4, 6, + } }, + { TileID.HallowedPlants2, new HashSet() + { + // 5 is intentionally missing here because it is being skipped by vanilla + 2, 3, 4, 6, 7, + } }, + { TileID.JunglePlants2, new HashSet() + { + 9, 10, 11, 12, 13, 14, 15, 16, + } }, }; /// @@ -40,7 +82,7 @@ public class SendTileRectHandler : IPacketHandler /// Maps TileIDs to Tile Entity IDs. - /// Note: is empty at the time of writing, but entities are dynamically assigned their ID at initialize time + /// Note: is empty at the time of writing, but entities are dynamically assigned their ID at initialize time /// which is why we can use the _myEntityId field on each entity type /// public static Dictionary TileEntityIdToTileIdMap = new Dictionary @@ -139,24 +181,75 @@ internal void IterateTileRect(NetTile[,] tiles, bool[,] processed, GetDataHandle // and process them as a tile object if (newTile.Type < TileObjectData._data.Count && TileObjectData._data[newTile.Type] != null) { + // Verify that the changes are actually valid conceptually + // Many tiles that are never placed or modified using this packet are valid TileObjectData entries, which is the main attack vector for most exploits using this packet + if (Main.tile[realX, realY].type == newTile.Type) + { + switch (newTile.Type) + { + // Some individual cases might still allow crashing exploits, as the actual framing is not being checked here + // Doing so requires hard-coding the individual valid framing values and is a lot of effort + case TileID.ProjectilePressurePad: + case TileID.WirePipe: + case TileID.Traps: + case TileID.Candles: + case TileID.PeaceCandle: + case TileID.WaterCandle: + case TileID.PlatinumCandle: + case TileID.Firework: + case TileID.WaterFountain: + case TileID.BloodMoonMonolith: + case TileID.VoidMonolith: + case TileID.LunarMonolith: + case TileID.MusicBoxes: + case TileID.ArrowSign: + case TileID.PaintedArrowSign: + case TileID.Cannon: + case TileID.Campfire: + case TileID.Plants: + case TileID.MinecartTrack: + case TileID.ChristmasTree: + { + // Allowed changes + } + break; + default: + { + continue; + } + } + } + else + { + // Together with Flower Boots and Land Mine destruction, these are the only cases where a tile type is allowed to be modified + switch (newTile.Type) + { + case TileID.LogicSensor: + case TileID.FoodPlatter: + case TileID.WeaponsRack2: + case TileID.ItemFrame: + case TileID.HatRack: + case TileID.DisplayDoll: + case TileID.TeleportationPylon: + case TileID.TargetDummy: + { + // Allowed placements + } + break; + default: + { + continue; + } + } + } + data = TileObjectData._data[newTile.Type]; NetTile[,] newTiles; int objWidth = data.Width; int objHeight = data.Height; - int offsetY = 0; - - if (newTile.Type == TileID.TrapdoorClosed) - { - // Trapdoors can modify a 2x3 space. When it closes it will have leftover tiles either on top or bottom. - // If we don't update these tiles, the trapdoor gets confused and disappears. - // So we capture all 6 possible tiles and offset ourselves 1 tile above the closed trapdoor to capture the entire 2x3 area - objWidth = 2; - objHeight = 3; - offsetY = -1; - } // Ensure the tile object fits inside the rect before processing it - if (!DoesTileObjectFitInTileRect(x, y, objWidth, objHeight, width, length, offsetY, processed)) + if (!DoesTileObjectFitInTileRect(x, y, objWidth, objHeight, width, length, processed)) { continue; } @@ -167,11 +260,11 @@ internal void IterateTileRect(NetTile[,] tiles, bool[,] processed, GetDataHandle { for (int j = 0; j < objHeight; j++) { - newTiles[i, j] = tiles[x + i, y + j + offsetY]; - processed[x + i, y + j + offsetY] = true; + newTiles[i, j] = tiles[x + i, y + j]; + processed[x + i, y + j] = true; } } - ProcessTileObject(newTile.Type, realX, realY + offsetY, objWidth, objHeight, newTiles, args); + ProcessTileObject(newTile.Type, realX, realY, objWidth, objHeight, newTiles, args); continue; } @@ -231,25 +324,41 @@ internal void ProcessSingleTile(int realX, int realY, NetTile newTile, byte rect { // Some boots allow growing flowers on grass. This process sends a 1x1 tile rect to grow the flowers // The rect size must be 1 and the player must have an accessory that allows growing flowers in order for this rect to be valid - if (rectWidth == 1 && rectLength == 1 && args.Player.Accessories.Any(a => a != null && FlowerBootItems.Contains(a.type))) + if (rectWidth == 1 && rectLength == 1 && WorldGen.InWorld(realX, realY + 1) && args.Player.Accessories.Any(a => a != null && FlowerBootItems.Contains(a.type))) { - ProcessFlowerBoots(realX, realY, newTile, args); + ProcessFlowerBoots(realX, realY, newTile); return; } ITile tile = Main.tile[realX, realY]; - if (tile.type == TileID.LandMine && !newTile.Active) + // Triggering a single land mine tile + if (rectWidth == 1 && rectLength == 1 && tile.type == TileID.LandMine && !newTile.Active) { UpdateServerTileState(tile, newTile, TileDataType.Tile); } - if (tile.type == TileID.WirePipe) + // Hammering a single junction box + if (rectWidth == 1 && rectLength == 1 && tile.type == TileID.WirePipe) { UpdateServerTileState(tile, newTile, TileDataType.Tile); } - ProcessConversionSpreads(Main.tile[realX, realY], newTile); + // Mowing a single grass tile: Grass -> GolfGrass OR HallowedGrass -> GolfGrassHallowed + if (rectWidth == 1 && rectLength == 1 && + ( + tile.type == TileID.Grass && newTile.Type == TileID.GolfGrass || + tile.type == TileID.HallowedGrass && newTile.Type == TileID.GolfGrassHallowed + )) + { + UpdateServerTileState(tile, newTile, TileDataType.Tile); + } + + // Conversion: only sends a 1x1 rect + if (rectWidth == 1 && rectLength == 1) + { + ProcessConversionSpreads(tile, newTile); + } // All other single tile updates should not be processed. } @@ -260,24 +369,22 @@ internal void ProcessSingleTile(int realX, int realY, NetTile newTile, byte rect /// The tile x position of the tile rect packet - this is where the flowers are intending to grow /// The tile y position of the tile rect packet - this is where the flowers are intending to grow /// The NetTile containing information about the flowers that are being grown - /// SendTileRectEventArgs containing event information - internal void ProcessFlowerBoots(int realX, int realY, NetTile newTile, GetDataHandlers.SendTileRectEventArgs args) + internal void ProcessFlowerBoots(int realX, int realY, NetTile newTile) { - // We need to get the tile below the tile rect to determine what grass types are allowed - if (!WorldGen.InWorld(realX, realY + 1)) - { - // If the tile below the tile rect isn't valid, we return here and don't update the server tile state - return; - } - - ITile tile = Main.tile[realX, realY + 1]; - if (!GrassToPlantMap.TryGetValue(tile.type, out List plantTiles) && !plantTiles.Contains(newTile.Type)) + ITile tile = Main.tile[realX, realY]; + // Ensure that: + // - the placed plant is valid for the grass below + // - the target tile is empty + // - and the placed plant has valid framing (style * 18 = FrameX) + if ( + FlowerBootPlantToGrassMap.TryGetValue(newTile.Type, out HashSet grassTiles) && + !tile.active() && + grassTiles.Contains(Main.tile[realX, realY + 1].type) && + FlowerBootPlantToStyleMap[newTile.Type].Contains((ushort)(newTile.FrameX / 18)) + ) { - // If the tile below the tile rect isn't a valid plant tile (eg grass) then we don't update the server tile state - return; + UpdateServerTileState(tile, newTile, TileDataType.Tile); } - - UpdateServerTileState(Main.tile[realX, realY], newTile, TileDataType.Tile); } /// @@ -446,15 +553,9 @@ static bool ShouldSkipProcessing(GetDataHandlers.SendTileRectEventArgs args) return true; } - var rectSize = args.Width * args.Length; - if (rectSize > TShock.Config.Settings.TileRectangleSizeThreshold) + if (args.Width > 4 || args.Length > 4) // as of 1.4.3.6 this is the biggest size the client will send in any case { TShock.Log.ConsoleDebug("Bouncer / SendTileRect rejected from non-vanilla tilemod from {0}", args.Player.Name); - if (TShock.Config.Settings.KickOnTileRectangleSizeThresholdBroken) - { - args.Player.Kick("Unexpected tile threshold reached"); - } - return true; } @@ -484,24 +585,16 @@ static bool ShouldSkipProcessing(GetDataHandlers.SendTileRectEventArgs args) /// /// /// - /// /// /// - static bool DoesTileObjectFitInTileRect(int x, int y, int width, int height, short rectWidth, short rectLength, int offsetY, bool[,] processed) + static bool DoesTileObjectFitInTileRect(int x, int y, int width, int height, short rectWidth, short rectLength, bool[,] processed) { - // If the starting y position of this tile object is at (x, 0) and the y offset is negative, we'll be accessing tiles outside the rect - if (y + offsetY < 0) - { - TShock.Log.ConsoleDebug("Bouncer / SendTileRectHandler - rejected tile object because object dimensions fall outside the tile rect (negative y value)"); - return false; - } - - if (x + width > rectWidth || y + height + offsetY > rectLength) + if (x + width > rectWidth || y + height > rectLength) { // This is ugly, but we want to mark all these tiles as processed so that we're not hitting this check multiple times for one dodgy tile object for (int i = x; i < rectWidth; i++) { - for (int j = Math.Max(0, y + offsetY); j < rectLength; j++) // This is also ugly. Using Math.Max to make sure y + offsetY >= 0 + for (int j = y; j < rectLength; j++) { processed[i, j] = true; }