Skip to content

Commit

Permalink
Merge remote-tracking branch 'punchfix/h/a-new-branch' into h/punchre…
Browse files Browse the repository at this point in the history 
…ady-fixes
  • Loading branch information
@hakusaro
hakusaro committed Aug 17, 2022
2 parents 2d09565 + 0ce7c94 commit 39383c9
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 67 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <type>` 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)
Expand Down
12 changes: 12 additions & 0 deletions TShockAPI/Bouncer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
8 changes: 0 additions & 8 deletions TShockAPI/Configuration/TShockConfig.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,14 +445,6 @@ public class TShockSettings
[Description("Whether or not to kick users when they surpass the HealOther threshold.")]
public bool KickOnHealOtherThresholdBroken = false;

/// <summary>Disables a player if this number of tiles is present in a Tile Rectangle packet</summary>
[Description("Disables a player if this number of tiles is present in a Tile Rectangle packet")]
public int TileRectangleSizeThreshold = 50;

/// <summary>Whether or not to kick users when they surpass the TileRectangleSize threshold.</summary>
[Description("Whether or not to kick users when they surpass the TileRectangleSize threshold.")]
public bool KickOnTileRectangleSizeThresholdBroken = false;

/// <summary>Whether or not the server should suppress build permission failure warnings from regions, spawn point, or server edit failure.</summary>
[Description("Whether or not the server should suppress build permission failure warnings from regions, spawn point, or server edit failure.")]
public bool SuppressPermissionFailureNotices = false;
Expand Down
211 changes: 152 additions & 59 deletions TShockAPI/Handlers/SendTileRectHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,55 @@ namespace TShockAPI.Handlers
public class SendTileRectHandler : IPacketHandler<GetDataHandlers.SendTileRectEventArgs>
{
/// <summary>
/// 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
/// </summary>
public static Dictionary<ushort, List<ushort>> GrassToPlantMap = new Dictionary<ushort, List<ushort>>
private static readonly Dictionary<ushort, HashSet<ushort>> FlowerBootPlantToGrassMap = new Dictionary<ushort, HashSet<ushort>>
{
{ TileID.Grass, new List<ushort> { TileID.Plants, TileID.Plants2 } },
{ TileID.HallowedGrass, new List<ushort> { TileID.HallowedPlants, TileID.HallowedPlants2 } },
{ TileID.JungleGrass, new List<ushort> { TileID.JunglePlants, TileID.JunglePlants2 } }
{ TileID.Plants, new HashSet<ushort>()
{
TileID.Grass, TileID.GolfGrass
} },
{ TileID.HallowedPlants, new HashSet<ushort>()
{
TileID.HallowedGrass, TileID.GolfGrassHallowed
} },
{ TileID.HallowedPlants2, new HashSet<ushort>()
{
TileID.HallowedGrass, TileID.GolfGrassHallowed
} },
{ TileID.JunglePlants2, new HashSet<ushort>()
{
TileID.JungleGrass
} },
};

/// <summary>
/// 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`
/// </summary>
private static readonly Dictionary<ushort, HashSet<ushort>> FlowerBootPlantToStyleMap = new Dictionary<ushort, HashSet<ushort>>()
{
{ TileID.Plants, new HashSet<ushort>()
{
// 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<ushort>()
{
// 5 is intentionally missing here because it is being skipped by vanilla
4, 6,
} },
{ TileID.HallowedPlants2, new HashSet<ushort>()
{
// 5 is intentionally missing here because it is being skipped by vanilla
2, 3, 4, 6, 7,
} },
{ TileID.JunglePlants2, new HashSet<ushort>()
{
9, 10, 11, 12, 13, 14, 15, 16,
} },
};

/// <summary>
Expand All @@ -40,7 +82,7 @@ public class SendTileRectHandler : IPacketHandler<GetDataHandlers.SendTileRectEv

/// <summary>
/// Maps TileIDs to Tile Entity IDs.
/// Note: <see cref="Terraria.ID.TileEntityID"/> is empty at the time of writing, but entities are dynamically assigned their ID at initialize time
/// Note: <see cref="TileEntityID"/> 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
/// </summary>
public static Dictionary<int, int> TileEntityIdToTileIdMap = new Dictionary<int, int>
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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.
}
Expand All @@ -260,24 +369,22 @@ internal void ProcessSingleTile(int realX, int realY, NetTile newTile, byte rect
/// <param name="realX">The tile x position of the tile rect packet - this is where the flowers are intending to grow</param>
/// <param name="realY">The tile y position of the tile rect packet - this is where the flowers are intending to grow</param>
/// <param name="newTile">The NetTile containing information about the flowers that are being grown</param>
/// <param name="args">SendTileRectEventArgs containing event information</param>
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<ushort> 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<ushort> 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);
}

/// <summary>
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -484,24 +585,16 @@ static bool ShouldSkipProcessing(GetDataHandlers.SendTileRectEventArgs args)
/// <param name="height"></param>
/// <param name="rectWidth"></param>
/// <param name="rectLength"></param>
/// <param name="offsetY"></param>
/// <param name="processed"></param>
/// <returns></returns>
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;
}
Expand Down

0 comments on commit 39383c9

Please sign in to comment.