From c6dad11a1cc176ded14b4b6b7ecd4676b6689680 Mon Sep 17 00:00:00 2001 From: quake1337 <3310937+bartico6@users.noreply.github.com> Date: Fri, 21 May 2021 09:53:33 +0200 Subject: [PATCH 1/6] Initial patch for advisory-1. - Remove unused fields from NetTile - Apply data from packets selectively based on approving context. --- TShockAPI/Handlers/SendTileRectHandler.cs | 93 ++++++++++++++++++++--- TShockAPI/Net/NetTile.cs | 4 - 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/TShockAPI/Handlers/SendTileRectHandler.cs b/TShockAPI/Handlers/SendTileRectHandler.cs index 2e52baf7c..d0e0b5471 100644 --- a/TShockAPI/Handlers/SendTileRectHandler.cs +++ b/TShockAPI/Handlers/SendTileRectHandler.cs @@ -238,12 +238,12 @@ internal void ProcessSingleTile(int realX, int realY, NetTile newTile, byte rect if (tile.type == TileID.LandMine && !newTile.Active) { - UpdateServerTileState(tile, newTile); + UpdateServerTileState(tile, newTile, TileDataType.Tile); } if (tile.type == TileID.WirePipe) { - UpdateServerTileState(tile, newTile); + UpdateServerTileState(tile, newTile, TileDataType.Tile); } ProcessConversionSpreads(Main.tile[realX, realY], newTile); @@ -274,7 +274,7 @@ internal void ProcessFlowerBoots(int realX, int realY, NetTile newTile, GetDataH return; } - UpdateServerTileState(Main.tile[realX, realY], newTile); + UpdateServerTileState(Main.tile[realX, realY], newTile, TileDataType.Tile); } /// @@ -295,8 +295,14 @@ internal void ProcessConversionSpreads(ITile tile, NetTile newTile) TileID.Sets.Conversion.HardenedSand[tile.type] && TileID.Sets.Conversion.HardenedSand[newTile.Type] || TileID.Sets.Conversion.Thorn[tile.type] && TileID.Sets.Conversion.Thorn[newTile.Type] || TileID.Sets.Conversion.Moss[tile.type] && TileID.Sets.Conversion.Moss[newTile.Type] || - TileID.Sets.Conversion.MossBrick[tile.type] && TileID.Sets.Conversion.MossBrick[newTile.Type] || - WallID.Sets.Conversion.Stone[tile.wall] && WallID.Sets.Conversion.Stone[newTile.Wall] || + TileID.Sets.Conversion.MossBrick[tile.type] && TileID.Sets.Conversion.MossBrick[newTile.Type] + ) + { + TShock.Log.ConsoleDebug("Bouncer / SendTileRect processing a tile conversion update - [{0}] -> [{1}]", tile.type, newTile.Type); + UpdateServerTileState(tile, newTile, TileDataType.Tile); + } + + if(WallID.Sets.Conversion.Stone[tile.wall] && WallID.Sets.Conversion.Stone[newTile.Wall] || WallID.Sets.Conversion.Grass[tile.wall] && WallID.Sets.Conversion.Grass[newTile.Wall] || WallID.Sets.Conversion.Sandstone[tile.wall] && WallID.Sets.Conversion.Sandstone[newTile.Wall] || WallID.Sets.Conversion.HardenedSand[tile.wall] && WallID.Sets.Conversion.HardenedSand[newTile.Wall] || @@ -307,8 +313,75 @@ internal void ProcessConversionSpreads(ITile tile, NetTile newTile) WallID.Sets.Conversion.NewWall4[tile.wall] && WallID.Sets.Conversion.NewWall4[newTile.Wall] ) { - TShock.Log.ConsoleDebug("Bouncer / SendTileRect processing a conversion update - [{0}|{1}] -> [{2}|{3}]", tile.type, tile.wall, newTile.Type, newTile.Wall); - UpdateServerTileState(tile, newTile); + TShock.Log.ConsoleDebug("Bouncer / SendTileRect processing a wall conversion update - [{0}] -> [{1}]", tile.wall, newTile.Wall); + UpdateServerTileState(tile, newTile, TileDataType.Wall); + } + } + + /// + /// Updates a single tile's world state with a set of changes from the networked tile state + /// + /// The tile to update + /// The NetTile containing the change + /// The type of data to merge into world state + public static void UpdateServerTileState(ITile tile, NetTile newTile, TileDataType data) + { + if ((data & TileDataType.Tile) != 0) + { + tile.active(newTile.Active); + tile.type = newTile.Type; + + if (newTile.FrameImportant) + { + tile.frameX = newTile.FrameX; + tile.frameY = newTile.FrameY; + } + else if (tile.type != newTile.Type || !tile.active()) + { + tile.frameX = -1; + tile.frameY = -1; + } + } + + if ((data & TileDataType.Wall) != 0) + { + tile.wall = newTile.Wall; + } + + if ((data & TileDataType.TilePaint) != 0) + { + tile.color(newTile.TileColor); + } + + if((data & TileDataType.WallPaint) != 0) + { + tile.wallColor(newTile.WallColor); + } + + if((data & TileDataType.Liquid) != 0) + { + tile.liquid = newTile.Liquid; + tile.liquidType(newTile.LiquidType); + } + + if((data & TileDataType.Slope) != 0) + { + tile.halfBrick(newTile.IsHalf); + tile.slope((byte)((newTile.Slope ? 1 : 0) + (newTile.Slope2 ? 2 : 0) + (newTile.Slope3 ? 4 : 0))); + } + + if((data & TileDataType.Wiring) != 0) + { + tile.wire(newTile.Wire); + tile.wire2(newTile.Wire2); + tile.wire3(newTile.Wire3); + tile.wire4(newTile.Wire4); + } + + if((data & TileDataType.Actuator) != 0) + { + tile.actuator(newTile.IsActuator); + tile.inActive(newTile.Inactive); } } @@ -376,7 +449,7 @@ public static void UpdateServerTileState(ITile tile, NetTile newTile) } /// - /// Performs on multiple tiles + /// Performs on multiple tiles /// /// /// @@ -389,7 +462,7 @@ public static void UpdateMultipleServerTileStates(int x, int y, int width, int h { for (int j = 0; j < height; j++) { - UpdateServerTileState(Main.tile[x + i, y + j], newTiles[i, j]); + UpdateServerTileState(Main.tile[x + i, y + j], newTiles[i, j], TileDataType.Tile); } } } @@ -553,7 +626,7 @@ public static void DisplayTileSetInGame(short tileX, short tileY, byte width, by UpdateServerTileState(Main.tile[tileX + x, tileY + y], newTiles[x, y]); } //Add a line of dirt blocks at the bottom for safety - UpdateServerTileState(Main.tile[tileX + x, tileY + height], new NetTile { Active = true, Type = 0 }); + UpdateServerTileState(Main.tile[tileX + x, tileY + height], new NetTile { Active = true, Type = 0 }, TileDataType.All); } player.SendTileRect(tileX, tileY, width, height); diff --git a/TShockAPI/Net/NetTile.cs b/TShockAPI/Net/NetTile.cs index 62b362f97..eba0b0005 100644 --- a/TShockAPI/Net/NetTile.cs +++ b/TShockAPI/Net/NetTile.cs @@ -37,8 +37,6 @@ public class NetTile : IPackable public bool Wire2 { get; set; } public bool Wire3 { get; set; } public bool Wire4 { get; set; } - public byte HalfBrick { get; set; } - public byte Actuator { get; set; } public bool Inactive { get; set; } public bool IsHalf { get; set; } public bool IsActuator { get; set; } @@ -85,8 +83,6 @@ public NetTile() Wire2 = false; Wire3 = false; Wire4 = false; - HalfBrick = 0; - Actuator = 0; Inactive = false; TileColor = 0; WallColor = 0; From ccf5a422ffffe5129fcdbcd3639abeb50cad235d Mon Sep 17 00:00:00 2001 From: quake1337 <3310937+bartico6@users.noreply.github.com> Date: Fri, 21 May 2021 11:58:33 +0200 Subject: [PATCH 2/6] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b14bd8a3..53d041c25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin * If there is no section called "Upcoming changes" below this line, please add one with `## Upcoming changes` as the first line, and then a bulleted item directly after with the first change. ## Upcoming changes + +## TShock 4.5.3 * Added permissions for using Teleportation Potions, Magic Conch, and Demon Conch. (@drunderscore) * `tshock.tp.tppotion`, `tshock.tp.magicconch`, and `tshock.tp.demonconch` respectively. * Updated HealOtherPlayer damage check to make more sense by respecting `ignoredamagecap` permission. (@moisterrific) @@ -22,6 +24,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin * The default MOTD is now prettier. The MOTD format can now contain `%specifier%` to send the command specifier. (@moisterrific) * The buff commands now support `-1` as a time option to set buffs that last 415 days (the maximum buff time the game supports). (@moisterrific) * TShock defaults to saving backups every 10 minutes, and defaults to keeping backups for 4 hours. (@hakusaro) +* Fix oversight & exploit allowing crafted SendTileRectangle packets to perform large-scale world griefing (@bartico6) ## TShock 4.5.2 * Added preliminary support for Terraria 1.4.2.2. (@hakusaro) From 658c714ac5d1b354dd21c102b80e53b2665f2e35 Mon Sep 17 00:00:00 2001 From: quake1337 <3310937+bartico6@users.noreply.github.com> Date: Fri, 21 May 2021 12:13:06 +0200 Subject: [PATCH 3/6] Remove old UpdateServerTileState call, as the new one supersedes it --- TShockAPI/Handlers/SendTileRectHandler.cs | 65 +---------------------- 1 file changed, 1 insertion(+), 64 deletions(-) diff --git a/TShockAPI/Handlers/SendTileRectHandler.cs b/TShockAPI/Handlers/SendTileRectHandler.cs index d0e0b5471..e737b6fcf 100644 --- a/TShockAPI/Handlers/SendTileRectHandler.cs +++ b/TShockAPI/Handlers/SendTileRectHandler.cs @@ -385,69 +385,6 @@ public static void UpdateServerTileState(ITile tile, NetTile newTile, TileDataTy } } - /// - /// Updates a single tile's world state with a change from the tile rect packet - /// - /// The tile to update - /// The NetTile containing the change - public static void UpdateServerTileState(ITile tile, NetTile newTile) - { - tile.active(newTile.Active); - tile.type = newTile.Type; - - if (newTile.FrameImportant) - { - tile.frameX = newTile.FrameX; - tile.frameY = newTile.FrameY; - } - - if (newTile.HasWall) - { - tile.wall = newTile.Wall; - } - - if (newTile.HasLiquid) - { - tile.liquid = newTile.Liquid; - tile.liquidType(newTile.LiquidType); - } - - tile.wire(newTile.Wire); - tile.wire2(newTile.Wire2); - tile.wire3(newTile.Wire3); - tile.wire4(newTile.Wire4); - - tile.halfBrick(newTile.IsHalf); - - if (newTile.HasColor) - { - tile.color(newTile.TileColor); - } - - if (newTile.HasWallColor) - { - tile.wallColor(newTile.WallColor); - } - - byte slope = 0; - if (newTile.Slope) - { - slope += 1; - } - if (newTile.Slope2) - { - slope += 2; - } - if (newTile.Slope3) - { - slope += 4; - } - - tile.slope(slope); - - TShock.Log.ConsoleDebug("Bouncer / SendTileRect updated a tile from type {0} to {1}", tile.type, newTile.Type); - } - /// /// Performs on multiple tiles /// @@ -623,7 +560,7 @@ public static void DisplayTileSetInGame(short tileX, short tileY, byte width, by { for (int y = 0; y < height; y++) { - UpdateServerTileState(Main.tile[tileX + x, tileY + y], newTiles[x, y]); + UpdateServerTileState(Main.tile[tileX + x, tileY + y], newTiles[x, y], TileDataType.All); } //Add a line of dirt blocks at the bottom for safety UpdateServerTileState(Main.tile[tileX + x, tileY + height], new NetTile { Active = true, Type = 0 }, TileDataType.All); From 817dfe26fc056e3e56a7694f013a1a6c3035f41f Mon Sep 17 00:00:00 2001 From: quake1337 <3310937+bartico6@users.noreply.github.com> Date: Fri, 21 May 2021 13:13:06 +0200 Subject: [PATCH 4/6] Address feedback from @hakusaro about style & documentation --- CHANGELOG.md | 2 +- TShockAPI/Handlers/SendTileRectHandler.cs | 26 +++++++++++--------- TShockAPI/Net/NetTile.cs | 30 ++++++++++++++++++----- 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 140bd4ba9..ca483e9b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin * The buff commands now support `-1` as a time option to set buffs that last 415 days (the maximum buff time the game supports). (@moisterrific) * TShock defaults to saving backups every 10 minutes, and defaults to keeping backups for 4 hours. (@hakusaro) * Updated SSC bypass messaging. Now, when you connect, you're told if you're bypassing SSC. Console logging has been improved to warn when players are not being saved due to the bypass SSC permission. To turn this warning off, change `WarnPlayersAboutBypassPermission` to `false` in the `sscconfig.json` file. (@hakusaro) -* Fix oversight & exploit allowing crafted SendTileRectangle packets to perform large-scale world griefing (@bartico6) +* Fix oversight & exploit allowing specially crafted SendTileRectangle packets to perform large-scale world griefing (@bartico6) ## TShock 4.5.2 * Added preliminary support for Terraria 1.4.2.2. (@hakusaro) diff --git a/TShockAPI/Handlers/SendTileRectHandler.cs b/TShockAPI/Handlers/SendTileRectHandler.cs index e737b6fcf..09fbb25c3 100644 --- a/TShockAPI/Handlers/SendTileRectHandler.cs +++ b/TShockAPI/Handlers/SendTileRectHandler.cs @@ -323,10 +323,13 @@ internal void ProcessConversionSpreads(ITile tile, NetTile newTile) /// /// The tile to update /// The NetTile containing the change - /// The type of data to merge into world state - public static void UpdateServerTileState(ITile tile, NetTile newTile, TileDataType data) + /// The type of data to merge into world state + public static void UpdateServerTileState(ITile tile, NetTile newTile, TileDataType updateType) { - if ((data & TileDataType.Tile) != 0) + //This logic (updateType & TDT.Tile) != 0 is the way Terraria does it (see: Tile.cs/Clear(TileDataType)) + //& is not a typo - we're performing a binary AND test to see if a given flag is set. + + if ((updateType & TileDataType.Tile) != 0) { tile.active(newTile.Active); tile.type = newTile.Type; @@ -338,39 +341,40 @@ public static void UpdateServerTileState(ITile tile, NetTile newTile, TileDataTy } else if (tile.type != newTile.Type || !tile.active()) { + //This is vanilla logic - if the tile changed types (or wasn't active) the frame values might not be valid - so we reset them to -1. tile.frameX = -1; tile.frameY = -1; } } - if ((data & TileDataType.Wall) != 0) + if ((updateType & TileDataType.Wall) != 0) { tile.wall = newTile.Wall; } - if ((data & TileDataType.TilePaint) != 0) + if ((updateType & TileDataType.TilePaint) != 0) { tile.color(newTile.TileColor); } - if((data & TileDataType.WallPaint) != 0) + if ((updateType & TileDataType.WallPaint) != 0) { tile.wallColor(newTile.WallColor); } - if((data & TileDataType.Liquid) != 0) + if ((updateType & TileDataType.Liquid) != 0) { tile.liquid = newTile.Liquid; tile.liquidType(newTile.LiquidType); } - if((data & TileDataType.Slope) != 0) + if ((updateType & TileDataType.Slope) != 0) { tile.halfBrick(newTile.IsHalf); - tile.slope((byte)((newTile.Slope ? 1 : 0) + (newTile.Slope2 ? 2 : 0) + (newTile.Slope3 ? 4 : 0))); + tile.slope(newTile.Slope); } - if((data & TileDataType.Wiring) != 0) + if ((updateType & TileDataType.Wiring) != 0) { tile.wire(newTile.Wire); tile.wire2(newTile.Wire2); @@ -378,7 +382,7 @@ public static void UpdateServerTileState(ITile tile, NetTile newTile, TileDataTy tile.wire4(newTile.Wire4); } - if((data & TileDataType.Actuator) != 0) + if ((updateType & TileDataType.Actuator) != 0) { tile.actuator(newTile.IsActuator); tile.inActive(newTile.Inactive); diff --git a/TShockAPI/Net/NetTile.cs b/TShockAPI/Net/NetTile.cs index eba0b0005..18033c2b9 100644 --- a/TShockAPI/Net/NetTile.cs +++ b/TShockAPI/Net/NetTile.cs @@ -42,10 +42,29 @@ public class NetTile : IPackable public bool IsActuator { get; set; } public byte TileColor { get; set; } public byte WallColor { get; set; } - public bool Slope { get; set; } + public bool Slope1 { get; set; } public bool Slope2 { get; set; } public bool Slope3 { get; set; } + public byte Slope + { + get + { + byte sl = 0; + + if (Slope1) + sl += 1; + + if (Slope2) + sl += 2; + + if (Slope3) + sl += 4; + + return sl; + } + } + public bool HasColor { get { return TileColor > 0; } @@ -87,8 +106,9 @@ public NetTile() TileColor = 0; WallColor = 0; Lighted = false; - Slope = false; + Slope1 = false; Slope2 = false; + Slope3 = false; } public NetTile(Stream stream) @@ -120,9 +140,7 @@ public void Pack(Stream stream) bits[6] = true; if (Inactive) - { bits[7] = true; - } stream.WriteInt8((byte) bits); @@ -140,7 +158,7 @@ public void Pack(Stream stream) if (HasWallColor) bits[3] = true; - if (Slope) + if (Slope1) bits[4] = true; if (Slope2) @@ -191,7 +209,7 @@ public void Unpack(Stream stream) Wire2 = flags2[0]; Wire3 = flags2[1]; - Slope = flags2[4]; + Slope1 = flags2[4]; Slope2 = flags2[5]; Slope3 = flags2[6]; Wire4 = flags2[7]; From b1820c15166c2c5d8432cb001be39424fba94e5c Mon Sep 17 00:00:00 2001 From: quake1337 <3310937+bartico6@users.noreply.github.com> Date: Fri, 21 May 2021 13:54:40 +0200 Subject: [PATCH 5/6] Fix spaces lol --- TShockAPI/Handlers/SendTileRectHandler.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/TShockAPI/Handlers/SendTileRectHandler.cs b/TShockAPI/Handlers/SendTileRectHandler.cs index 09fbb25c3..b39556185 100644 --- a/TShockAPI/Handlers/SendTileRectHandler.cs +++ b/TShockAPI/Handlers/SendTileRectHandler.cs @@ -1,12 +1,15 @@ using OTAPI.Tile; + using System; using System.Collections.Generic; using System.Linq; + using Terraria; using Terraria.DataStructures; using Terraria.GameContent.Tile_Entities; using Terraria.ID; using Terraria.ObjectData; + using TShockAPI.Net; namespace TShockAPI.Handlers @@ -198,7 +201,7 @@ internal void ProcessTileObject(int tileType, int realX, int realY, int width, i TShock.Log.ConsoleDebug("Bouncer / SendTileRect rejected from no permission for tile object from {0}", args.Player.Name); return; } - + if (TShock.TileBans.TileIsBanned((short)tileType)) { TShock.Log.ConsoleDebug("Bouncer / SendTileRect rejected for banned tile"); @@ -302,7 +305,7 @@ internal void ProcessConversionSpreads(ITile tile, NetTile newTile) UpdateServerTileState(tile, newTile, TileDataType.Tile); } - if(WallID.Sets.Conversion.Stone[tile.wall] && WallID.Sets.Conversion.Stone[newTile.Wall] || + if (WallID.Sets.Conversion.Stone[tile.wall] && WallID.Sets.Conversion.Stone[newTile.Wall] || WallID.Sets.Conversion.Grass[tile.wall] && WallID.Sets.Conversion.Grass[newTile.Wall] || WallID.Sets.Conversion.Sandstone[tile.wall] && WallID.Sets.Conversion.Sandstone[newTile.Wall] || WallID.Sets.Conversion.HardenedSand[tile.wall] && WallID.Sets.Conversion.HardenedSand[newTile.Wall] || From 02b4bf79733875bd4c41f326d7ae66f4c4b360bd Mon Sep 17 00:00:00 2001 From: quake1337 <3310937+bartico6@users.noreply.github.com> Date: Fri, 21 May 2021 14:18:28 +0200 Subject: [PATCH 6/6] Update CHANGELOG.md Address @hakusaro's suggestion for the changelog. Co-authored-by: Lucas Nicodemus --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca483e9b9..781c0e7d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin * The buff commands now support `-1` as a time option to set buffs that last 415 days (the maximum buff time the game supports). (@moisterrific) * TShock defaults to saving backups every 10 minutes, and defaults to keeping backups for 4 hours. (@hakusaro) * Updated SSC bypass messaging. Now, when you connect, you're told if you're bypassing SSC. Console logging has been improved to warn when players are not being saved due to the bypass SSC permission. To turn this warning off, change `WarnPlayersAboutBypassPermission` to `false` in the `sscconfig.json` file. (@hakusaro) -* Fix oversight & exploit allowing specially crafted SendTileRectangle packets to perform large-scale world griefing (@bartico6) +* Fix oversight & exploit allowing specially crafted SendTileRectangle packets to perform large-scale world griefing. In addition, `NetTile.Slope` is now the native value (byte), and accessor methods `Slope1`, `Slope2`, and `Slope3` can be used to get the old style of values out. `HalfBrick` and `Actuator` were removed from `NetTile` because these were initialized to zero and never changed or used. (@bartico6) ## TShock 4.5.2 * Added preliminary support for Terraria 1.4.2.2. (@hakusaro)