From b316ba82007c3cf6108244b769da62e2c989ff8d Mon Sep 17 00:00:00 2001 From: Lucas Nicodemus Date: Thu, 21 Dec 2017 10:57:46 -0700 Subject: [PATCH 1/7] Move inventory stack hack detection to TSPlayer Only called in one method, the stack hack detection can move to TSPlayer as it only ever operates on one player. In a future commit, this will replace the stack hack detection OnSecondUpdate() and also set the disabled flag if a player has a hacked stack when called. --- TShockAPI/GetDataHandlers.cs | 4 +- TShockAPI/TSPlayer.cs | 206 +++++++++++++++++++++++++++++++++++ TShockAPI/TShock.cs | 197 --------------------------------- 3 files changed, 209 insertions(+), 198 deletions(-) diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index e5336967..d8eb344d 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -1916,7 +1916,9 @@ namespace TShockAPI if (!args.Player.HasPermission(Permissions.ignorestackhackdetection)) { - TShock.HackedInventory(args.Player); + // TODO: When checkignores gets merged, this needs to set the hacked item stack flag + // and get moved to bouncer + args.Player.HasHackedItemStacks(true); } if (TShock.Utils.ActivePlayers() + 1 > TShock.Config.MaxSlots && diff --git a/TShockAPI/TSPlayer.cs b/TShockAPI/TSPlayer.cs index 868f20b2..fcc29b04 100644 --- a/TShockAPI/TSPlayer.cs +++ b/TShockAPI/TSPlayer.cs @@ -300,6 +300,212 @@ namespace TShockAPI return IgnoreActionsForInventory != "none" || IgnoreActionsForCheating != "none" || IgnoreActionsForDisabledArmor != "none" || IgnoreActionsForClearingTrashCan || !IsLoggedIn && TShock.Config.RequireLogin; } + /// Checks to see if a player has hacked item stacks in their inventory, and messages them as it checks. + /// If the check should send a message to the player with the results of the check. + /// True if any stacks don't conform. + public bool HasHackedItemStacks(bool shouldSendMessage = false) + { + bool check = false; + + Item[] inventory = TPlayer.inventory; + Item[] armor = TPlayer.armor; + Item[] dye = TPlayer.dye; + Item[] miscEquips = TPlayer.miscEquips; + Item[] miscDyes = TPlayer.miscDyes; + Item[] piggy = TPlayer.bank.item; + Item[] safe = TPlayer.bank2.item; + Item[] forge = TPlayer.bank3.item; + Item trash = TPlayer.trashItem; + for (int i = 0; i < NetItem.MaxInventory; i++) + { + if (i < NetItem.InventoryIndex.Item2) + { + //0-58 + Item item = new Item(); + if (inventory[i] != null && inventory[i].netID != 0) + { + item.netDefaults(inventory[i].netID); + item.Prefix(inventory[i].prefix); + item.AffixName(); + if (inventory[i].stack > item.maxStack) + { + check = true; + if (shouldSendMessage) + { + SendErrorMessage("Stack cheat detected. Remove item {0} ({1}) and then rejoin.", item.Name, inventory[i].stack); + } + } + } + } + else if (i < NetItem.ArmorIndex.Item2) + { + //59-78 + var index = i - NetItem.ArmorIndex.Item1; + Item item = new Item(); + if (armor[index] != null && armor[index].netID != 0) + { + item.netDefaults(armor[index].netID); + item.Prefix(armor[index].prefix); + item.AffixName(); + if (armor[index].stack > item.maxStack) + { + check = true; + if (shouldSendMessage) + { + SendErrorMessage("Stack cheat detected. Remove armor {0} ({1}) and then rejoin.", item.Name, armor[index].stack); + } + } + } + } + else if (i < NetItem.DyeIndex.Item2) + { + //79-88 + var index = i - NetItem.DyeIndex.Item1; + Item item = new Item(); + if (dye[index] != null && dye[index].netID != 0) + { + item.netDefaults(dye[index].netID); + item.Prefix(dye[index].prefix); + item.AffixName(); + if (dye[index].stack > item.maxStack) + { + check = true; + if (shouldSendMessage) + { + SendErrorMessage("Stack cheat detected. Remove dye {0} ({1}) and then rejoin.", item.Name, dye[index].stack); + } + } + } + } + else if (i < NetItem.MiscEquipIndex.Item2) + { + //89-93 + var index = i - NetItem.MiscEquipIndex.Item1; + Item item = new Item(); + if (miscEquips[index] != null && miscEquips[index].netID != 0) + { + item.netDefaults(miscEquips[index].netID); + item.Prefix(miscEquips[index].prefix); + item.AffixName(); + if (miscEquips[index].stack > item.maxStack) + { + check = true; + if (shouldSendMessage) + { + SendErrorMessage("Stack cheat detected. Remove item {0} ({1}) and then rejoin.", item.Name, miscEquips[index].stack); + } + } + } + } + else if (i < NetItem.MiscDyeIndex.Item2) + { + //93-98 + var index = i - NetItem.MiscDyeIndex.Item1; + Item item = new Item(); + if (miscDyes[index] != null && miscDyes[index].netID != 0) + { + item.netDefaults(miscDyes[index].netID); + item.Prefix(miscDyes[index].prefix); + item.AffixName(); + if (miscDyes[index].stack > item.maxStack) + { + check = true; + if (shouldSendMessage) + { + SendErrorMessage("Stack cheat detected. Remove item dye {0} ({1}) and then rejoin.", item.Name, miscDyes[index].stack); + } + } + } + } + else if (i < NetItem.PiggyIndex.Item2) + { + //98-138 + var index = i - NetItem.PiggyIndex.Item1; + Item item = new Item(); + if (piggy[index] != null && piggy[index].netID != 0) + { + item.netDefaults(piggy[index].netID); + item.Prefix(piggy[index].prefix); + item.AffixName(); + + if (piggy[index].stack > item.maxStack) + { + check = true; + if (shouldSendMessage) + { + SendErrorMessage("Stack cheat detected. Remove Piggy-bank item {0} ({1}) and then rejoin.", item.Name, piggy[index].stack); + } + } + } + } + else if (i < NetItem.SafeIndex.Item2) + { + //138-178 + var index = i - NetItem.SafeIndex.Item1; + Item item = new Item(); + if (safe[index] != null && safe[index].netID != 0) + { + item.netDefaults(safe[index].netID); + item.Prefix(safe[index].prefix); + item.AffixName(); + + if (safe[index].stack > item.maxStack) + { + check = true; + if (shouldSendMessage) + { + SendErrorMessage("Stack cheat detected. Remove Safe item {0} ({1}) and then rejoin.", item.Name, safe[index].stack); + } + } + } + } + else if (i < NetItem.TrashIndex.Item2) + { + //179-219 + Item item = new Item(); + if (trash != null && trash.netID != 0) + { + item.netDefaults(trash.netID); + item.Prefix(trash.prefix); + item.AffixName(); + + if (trash.stack > item.maxStack) + { + check = true; + if (shouldSendMessage) + { + SendErrorMessage("Stack cheat detected. Remove trash item {0} ({1}) and then rejoin.", item.Name, trash.stack); + } + } + } + } + else + { + //220 + var index = i - NetItem.ForgeIndex.Item1; + Item item = new Item(); + if (forge[index] != null && forge[index].netID != 0) + { + item.netDefaults(forge[index].netID); + item.Prefix(forge[index].prefix); + item.AffixName(); + + if (forge[index].stack > item.maxStack) + { + check = true; + if (shouldSendMessage) + { + SendErrorMessage("Stack cheat detected. Remove Defender's Forge item {0} ({1}) and then rejoin.", item.Name, forge[index].stack); + } + } + } + + } + } + + return check; + } + /// /// The player's server side inventory data. /// diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs index 9766b93d..503e8d01 100644 --- a/TShockAPI/TShock.cs +++ b/TShockAPI/TShock.cs @@ -1948,203 +1948,6 @@ namespace TShockAPI return Utils.Distance(value1, value2); } - /// HackedInventory - Checks to see if a user has a hacked inventory. In addition, messages players the result. - /// player - The TSPlayer object. - /// bool - True if the player has a hacked inventory. - public static bool HackedInventory(TSPlayer player) - { - bool check = false; - - Item[] inventory = player.TPlayer.inventory; - Item[] armor = player.TPlayer.armor; - Item[] dye = player.TPlayer.dye; - Item[] miscEquips = player.TPlayer.miscEquips; - Item[] miscDyes = player.TPlayer.miscDyes; - Item[] piggy = player.TPlayer.bank.item; - Item[] safe = player.TPlayer.bank2.item; - Item[] forge = player.TPlayer.bank3.item; - Item trash = player.TPlayer.trashItem; - for (int i = 0; i < NetItem.MaxInventory; i++) - { - if (i < NetItem.InventoryIndex.Item2) - { - //0-58 - Item item = new Item(); - if (inventory[i] != null && inventory[i].netID != 0) - { - item.netDefaults(inventory[i].netID); - item.Prefix(inventory[i].prefix); - item.AffixName(); - if (inventory[i].stack > item.maxStack) - { - check = true; - player.SendMessage( - String.Format("Stack cheat detected. Remove item {0} ({1}) and then rejoin", item.Name, inventory[i].stack), - Color.Cyan); - } - } - } - else if (i < NetItem.ArmorIndex.Item2) - { - //59-78 - var index = i - NetItem.ArmorIndex.Item1; - Item item = new Item(); - if (armor[index] != null && armor[index].netID != 0) - { - item.netDefaults(armor[index].netID); - item.Prefix(armor[index].prefix); - item.AffixName(); - if (armor[index].stack > item.maxStack) - { - check = true; - player.SendMessage( - String.Format("Stack cheat detected. Remove armor {0} ({1}) and then rejoin", item.Name, armor[index].stack), - Color.Cyan); - } - } - } - else if (i < NetItem.DyeIndex.Item2) - { - //79-88 - var index = i - NetItem.DyeIndex.Item1; - Item item = new Item(); - if (dye[index] != null && dye[index].netID != 0) - { - item.netDefaults(dye[index].netID); - item.Prefix(dye[index].prefix); - item.AffixName(); - if (dye[index].stack > item.maxStack) - { - check = true; - player.SendMessage( - String.Format("Stack cheat detected. Remove dye {0} ({1}) and then rejoin", item.Name, dye[index].stack), - Color.Cyan); - } - } - } - else if (i < NetItem.MiscEquipIndex.Item2) - { - //89-93 - var index = i - NetItem.MiscEquipIndex.Item1; - Item item = new Item(); - if (miscEquips[index] != null && miscEquips[index].netID != 0) - { - item.netDefaults(miscEquips[index].netID); - item.Prefix(miscEquips[index].prefix); - item.AffixName(); - if (miscEquips[index].stack > item.maxStack) - { - check = true; - player.SendMessage( - String.Format("Stack cheat detected. Remove item {0} ({1}) and then rejoin", item.Name, miscEquips[index].stack), - Color.Cyan); - } - } - } - else if (i < NetItem.MiscDyeIndex.Item2) - { - //93-98 - var index = i - NetItem.MiscDyeIndex.Item1; - Item item = new Item(); - if (miscDyes[index] != null && miscDyes[index].netID != 0) - { - item.netDefaults(miscDyes[index].netID); - item.Prefix(miscDyes[index].prefix); - item.AffixName(); - if (miscDyes[index].stack > item.maxStack) - { - check = true; - player.SendMessage( - String.Format("Stack cheat detected. Remove item dye {0} ({1}) and then rejoin", item.Name, miscDyes[index].stack), - Color.Cyan); - } - } - } - else if (i < NetItem.PiggyIndex.Item2) - { - //98-138 - var index = i - NetItem.PiggyIndex.Item1; - Item item = new Item(); - if (piggy[index] != null && piggy[index].netID != 0) - { - item.netDefaults(piggy[index].netID); - item.Prefix(piggy[index].prefix); - item.AffixName(); - - if (piggy[index].stack > item.maxStack) - { - check = true; - player.SendMessage( - String.Format("Stack cheat detected. Remove Piggy-bank item {0} ({1}) and then rejoin", item.Name, piggy[index].stack), - Color.Cyan); - } - } - } - else if (i < NetItem.SafeIndex.Item2) - { - //138-178 - var index = i - NetItem.SafeIndex.Item1; - Item item = new Item(); - if (safe[index] != null && safe[index].netID != 0) - { - item.netDefaults(safe[index].netID); - item.Prefix(safe[index].prefix); - item.AffixName(); - - if (safe[index].stack > item.maxStack) - { - check = true; - player.SendMessage( - String.Format("Stack cheat detected. Remove Safe item {0} ({1}) and then rejoin", item.Name, safe[index].stack), - Color.Cyan); - } - } - } - else if (i < NetItem.TrashIndex.Item2) - { - //179-219 - Item item = new Item(); - if (trash != null && trash.netID != 0) - { - item.netDefaults(trash.netID); - item.Prefix(trash.prefix); - item.AffixName(); - - if (trash.stack > item.maxStack) - { - check = true; - player.SendMessage( - String.Format("Stack cheat detected. Remove trash item {0} ({1}) and then rejoin", item.Name, trash.stack), - Color.Cyan); - } - } - } - else - { - //220 - var index = i - NetItem.ForgeIndex.Item1; - Item item = new Item(); - if (forge[index] != null && forge[index].netID != 0) - { - item.netDefaults(forge[index].netID); - item.Prefix(forge[index].prefix); - item.AffixName(); - - if (forge[index].stack > item.maxStack) - { - check = true; - player.SendMessage( - String.Format("Stack cheat detected. Remove Defender's Forge item {0} ({1}) and then rejoin", item.Name, forge[index].stack), - Color.Cyan); - } - } - - } - } - - return check; - } - /// OnConfigRead - Fired when the config file has been read. /// file - The config file object. public void OnConfigRead(ConfigFile file) From f9a1819e261f7d01d5bf5257ea244648efbb88f6 Mon Sep 17 00:00:00 2001 From: Lucas Nicodemus Date: Thu, 21 Dec 2017 17:16:57 -0700 Subject: [PATCH 2/7] Update grammar on stack cheat messages. --- TShockAPI/GetDataHandlers.cs | 4 +--- TShockAPI/TSPlayer.cs | 43 +++++++++++++++++++----------------- TShockAPI/TShock.cs | 11 ++------- 3 files changed, 26 insertions(+), 32 deletions(-) diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index 1f0002c4..9162acd2 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -1916,9 +1916,7 @@ namespace TShockAPI if (!args.Player.HasPermission(Permissions.ignorestackhackdetection)) { - // TODO: When checkignores gets merged, this needs to set the hacked item stack flag - // and get moved to bouncer - args.Player.HasHackedItemStacks(true); + args.Player.IsDisabledForStackDetection = args.Player.HasHackedItemStacks(true); } if (TShock.Utils.ActivePlayers() + 1 > TShock.Config.MaxSlots && diff --git a/TShockAPI/TSPlayer.cs b/TShockAPI/TSPlayer.cs index 5b1c59a7..23e187fb 100644 --- a/TShockAPI/TSPlayer.cs +++ b/TShockAPI/TSPlayer.cs @@ -308,10 +308,12 @@ namespace TShockAPI } /// Checks to see if a player has hacked item stacks in their inventory, and messages them as it checks. - /// If the check should send a message to the player with the results of the check. + /// If the check should send a message to the player with the results of the check. /// True if any stacks don't conform. public bool HasHackedItemStacks(bool shouldSendMessage = false) { + // Iterates through each inventory location a player has. + // This section is sub divided into number ranges for what each range of slots corresponds to. bool check = false; Item[] inventory = TPlayer.inventory; @@ -327,14 +329,15 @@ namespace TShockAPI { if (i < NetItem.InventoryIndex.Item2) { - //0-58 + // From above: this is slots 0-58 in the inventory. + // 0-58 Item item = new Item(); if (inventory[i] != null && inventory[i].netID != 0) { item.netDefaults(inventory[i].netID); item.Prefix(inventory[i].prefix); item.AffixName(); - if (inventory[i].stack > item.maxStack) + if (inventory[i].stack > item.maxStack || inventory[i].stack < 0) { check = true; if (shouldSendMessage) @@ -346,7 +349,7 @@ namespace TShockAPI } else if (i < NetItem.ArmorIndex.Item2) { - //59-78 + // 59-78 var index = i - NetItem.ArmorIndex.Item1; Item item = new Item(); if (armor[index] != null && armor[index].netID != 0) @@ -354,7 +357,7 @@ namespace TShockAPI item.netDefaults(armor[index].netID); item.Prefix(armor[index].prefix); item.AffixName(); - if (armor[index].stack > item.maxStack) + if (armor[index].stack > item.maxStack || armor[index].stack < 0) { check = true; if (shouldSendMessage) @@ -366,7 +369,7 @@ namespace TShockAPI } else if (i < NetItem.DyeIndex.Item2) { - //79-88 + // 79-88 var index = i - NetItem.DyeIndex.Item1; Item item = new Item(); if (dye[index] != null && dye[index].netID != 0) @@ -374,7 +377,7 @@ namespace TShockAPI item.netDefaults(dye[index].netID); item.Prefix(dye[index].prefix); item.AffixName(); - if (dye[index].stack > item.maxStack) + if (dye[index].stack > item.maxStack || dye[index].stack < 0) { check = true; if (shouldSendMessage) @@ -386,7 +389,7 @@ namespace TShockAPI } else if (i < NetItem.MiscEquipIndex.Item2) { - //89-93 + // 89-93 var index = i - NetItem.MiscEquipIndex.Item1; Item item = new Item(); if (miscEquips[index] != null && miscEquips[index].netID != 0) @@ -394,7 +397,7 @@ namespace TShockAPI item.netDefaults(miscEquips[index].netID); item.Prefix(miscEquips[index].prefix); item.AffixName(); - if (miscEquips[index].stack > item.maxStack) + if (miscEquips[index].stack > item.maxStack || miscEquips[index].stack < 0) { check = true; if (shouldSendMessage) @@ -406,7 +409,7 @@ namespace TShockAPI } else if (i < NetItem.MiscDyeIndex.Item2) { - //93-98 + // 93-98 var index = i - NetItem.MiscDyeIndex.Item1; Item item = new Item(); if (miscDyes[index] != null && miscDyes[index].netID != 0) @@ -414,7 +417,7 @@ namespace TShockAPI item.netDefaults(miscDyes[index].netID); item.Prefix(miscDyes[index].prefix); item.AffixName(); - if (miscDyes[index].stack > item.maxStack) + if (miscDyes[index].stack > item.maxStack || miscDyes[index].stack < 0) { check = true; if (shouldSendMessage) @@ -426,7 +429,7 @@ namespace TShockAPI } else if (i < NetItem.PiggyIndex.Item2) { - //98-138 + // 98-138 var index = i - NetItem.PiggyIndex.Item1; Item item = new Item(); if (piggy[index] != null && piggy[index].netID != 0) @@ -435,19 +438,19 @@ namespace TShockAPI item.Prefix(piggy[index].prefix); item.AffixName(); - if (piggy[index].stack > item.maxStack) + if (piggy[index].stack > item.maxStack || piggy[index].stack < 0) { check = true; if (shouldSendMessage) { - SendErrorMessage("Stack cheat detected. Remove Piggy-bank item {0} ({1}) and then rejoin.", item.Name, piggy[index].stack); + SendErrorMessage("Stack cheat detected. Remove piggy-bank item {0} ({1}) and then rejoin.", item.Name, piggy[index].stack); } } } } else if (i < NetItem.SafeIndex.Item2) { - //138-178 + // 138-178 var index = i - NetItem.SafeIndex.Item1; Item item = new Item(); if (safe[index] != null && safe[index].netID != 0) @@ -456,19 +459,19 @@ namespace TShockAPI item.Prefix(safe[index].prefix); item.AffixName(); - if (safe[index].stack > item.maxStack) + if (safe[index].stack > item.maxStack || safe[index].stack < 0) { check = true; if (shouldSendMessage) { - SendErrorMessage("Stack cheat detected. Remove Safe item {0} ({1}) and then rejoin.", item.Name, safe[index].stack); + SendErrorMessage("Stack cheat detected. Remove safe item {0} ({1}) and then rejoin.", item.Name, safe[index].stack); } } } } else if (i < NetItem.TrashIndex.Item2) { - //179-219 + // 179-219 Item item = new Item(); if (trash != null && trash.netID != 0) { @@ -488,7 +491,7 @@ namespace TShockAPI } else { - //220 + // 220 var index = i - NetItem.ForgeIndex.Item1; Item item = new Item(); if (forge[index] != null && forge[index].netID != 0) @@ -497,7 +500,7 @@ namespace TShockAPI item.Prefix(forge[index].prefix); item.AffixName(); - if (forge[index].stack > item.maxStack) + if (forge[index].stack > item.maxStack || forge[index].stack < 0) { check = true; if (shouldSendMessage) diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs index b5f758bc..d466a22a 100644 --- a/TShockAPI/TShock.cs +++ b/TShockAPI/TShock.cs @@ -1096,17 +1096,10 @@ namespace TShockAPI else if (!Main.ServerSideCharacter || (Main.ServerSideCharacter && player.IsLoggedIn)) { string check = "none"; - foreach (Item item in player.TPlayer.inventory) + if (!player.HasPermission(Permissions.ignorestackhackdetection)) { - if (!player.HasPermission(Permissions.ignorestackhackdetection) && (item.stack > item.maxStack || item.stack < 0) && - item.type != 0) - { - check = "Remove item " + item.Name + " (" + item.stack + ") exceeds max stack of " + item.maxStack; - player.SendErrorMessage(check); - break; - } + player.IsDisabledForStackDetection = player.HasHackedItemStacks(true); } - player.IsDisabledForStackDetection = true; check = "none"; // Please don't remove this for the time being; without it, players wearing banned equipment will only get debuffed once foreach (Item item in player.TPlayer.armor) From 390649aa56852728aa7d428da4b14ec6c5b0b04b Mon Sep 17 00:00:00 2001 From: Lucas Nicodemus Date: Thu, 21 Dec 2017 18:41:34 -0700 Subject: [PATCH 3/7] Move OnGetSection to Bouncer Bonus: Introduces a new GetDataHandledEventArgs for use in events that have players, data, and need to be handled. I was originally going to modify GetDataHandlerArgs, but that comes from the EventArgs class of args that isn't handled, and changing that to extend HandledEventArgs would imply we care or check to see if those are handled and we don't so we're stuck with this implementation for a teenie tiny bit. --- TShockAPI/Bouncer.cs | 23 +++++++++++++ TShockAPI/GetDataHandlers.cs | 63 +++++++++++++++++++++++++++++------- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/TShockAPI/Bouncer.cs b/TShockAPI/Bouncer.cs index 8e81d802..b0b6a427 100644 --- a/TShockAPI/Bouncer.cs +++ b/TShockAPI/Bouncer.cs @@ -42,6 +42,7 @@ namespace TShockAPI { // Setup hooks + GetDataHandlers.GetSection += OnGetSection; GetDataHandlers.PlaceItemFrame += OnPlaceItemFrame; GetDataHandlers.GemLockToggle += OnGemLockToggle; GetDataHandlers.PlaceTileEntity += OnPlaceTileEntity; @@ -64,6 +65,28 @@ namespace TShockAPI GetDataHandlers.TileEdit += OnTileEdit; } + internal void OnGetSection(object sender, GetDataHandlers.GetSectionEventArgs args) + { + if (args.Player.RequestedSection) + { + args.Handled = true; + return; + } + args.Player.RequestedSection = true; + + if (String.IsNullOrEmpty(args.Player.Name)) + { + TShock.Utils.ForceKick(args.Player, "Blank name.", true); + args.Handled = true; + return; + } + + if (!args.Player.HasPermission(Permissions.ignorestackhackdetection)) + { + args.Player.IsDisabledForStackDetection = args.Player.HasHackedItemStacks(true); + } + } + /// Fired when an item frame is placed for anti-cheat detection. /// The object that triggered the event. /// The packet arguments that the event has. diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index 9162acd2..60ace5df 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -58,6 +58,19 @@ namespace TShockAPI } } + /// + /// A custom HandledEventArgs that contains TShock's TSPlayer for the triggering uesr and the Terraria MP data stream. + /// Differentiated by GetDataHandlerArgs because it can be handled and responds to being handled. + /// + public class GetDataHandledEventArgs : HandledEventArgs + { + /// The TSPlayer that triggered the event. + public TSPlayer Player { get; set; } + + /// The raw MP packet data associated with the event. + public MemoryStream Data { get; set; } + } + public static class GetDataHandlers { private static Dictionary GetDataHandlerDelegates; @@ -1903,21 +1916,47 @@ namespace TShockAPI return true; } + /// The arguments to a GetSection packet. + public class GetSectionEventArgs : GetDataHandledEventArgs + { + /// The X position requested. Or -1 for spawn. + public int X { get; set; } + + /// The Y position requested. Or -1 for spawn. + public int Y { get; set; } + } + + /// The hook for a GetSection event. + public static HandlerList GetSection = new HandlerList(); + + /// Fires a GetSection event. + /// The TSPlayer that caused the GetSection. + /// The raw MP protocol data. + /// The x coordinate requested or -1 for spawn. + /// The y coordinate requested or -1 for spawn. + /// bool + private static bool OnGetSection(TSPlayer player, MemoryStream data, int x, int y) + { + if (GetSection == null) + return false; + + var args = new GetSectionEventArgs + { + Player = player, + Data = data, + X = x, + Y = y, + }; + + GetSection.Invoke(null, args); + return args.Handled; + } + private static bool HandleGetSection(GetDataHandlerArgs args) { - if (args.Player.RequestedSection) - return true; - args.Player.RequestedSection = true; - if (String.IsNullOrEmpty(args.Player.Name)) - { - TShock.Utils.ForceKick(args.Player, "Blank name.", true); - return true; - } - if (!args.Player.HasPermission(Permissions.ignorestackhackdetection)) - { - args.Player.IsDisabledForStackDetection = args.Player.HasHackedItemStacks(true); - } + if (OnGetSection(args.Player, args.Data, args.Data.ReadInt32(), args.Data.ReadInt32())) + return true; if (TShock.Utils.ActivePlayers() + 1 > TShock.Config.MaxSlots && !args.Player.HasPermission(Permissions.reservedslot)) From ef1486b78c97592b5cd6ceb551cc3eb6f6a67d51 Mon Sep 17 00:00:00 2001 From: Lucas Nicodemus Date: Thu, 21 Dec 2017 18:41:34 -0700 Subject: [PATCH 4/7] Move OnGetSection to Bouncer Bonus: Introduces a new GetDataHandledEventArgs for use in events that have players, data, and need to be handled. I was originally going to modify GetDataHandlerArgs, but that comes from the EventArgs class of args that isn't handled, and changing that to extend HandledEventArgs would imply we care or check to see if those are handled and we don't so we're stuck with this implementation for a teenie tiny bit. --- CHANGELOG.md | 1 + TShockAPI/Bouncer.cs | 23 +++++++++++++ TShockAPI/GetDataHandlers.cs | 62 +++++++++++++++++++++++++++++------- 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7286ea27..5149a73e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin * Added `TSPlayer.CheckIgnores()` and removed `TShock.CheckIgnores(TSPlayer)`. (@hakusaro) * Hooks inside TShock can now be registered with their `Register` method and can be prioritized according to the TShock HandlerList system. (@hakusaro) * Fix message requiring login not using the command specifier set in the config file. (@hakusaro) +* Fix stack hack detection being inconsistent between two different check points. Moved `TShock.HackedInventory` to `TSPlayer.HasHackedItemStacks`. Added `GetDataHandlers.GetDataHandledEventArgs` which is where most hooks will inherit from in the future. (@hakusaro) ## TShock 4.3.25 * Fixed a critical exploit in the Terraria protocol that could cause massive unpreventable world corruption as well as a number of other problems. Thanks to @bartico6 for reporting. Fixed by the efforts of @QuiCM, @hakusaro, and tips in the right directioon from @bartico6. diff --git a/TShockAPI/Bouncer.cs b/TShockAPI/Bouncer.cs index 8e81d802..b0b6a427 100644 --- a/TShockAPI/Bouncer.cs +++ b/TShockAPI/Bouncer.cs @@ -42,6 +42,7 @@ namespace TShockAPI { // Setup hooks + GetDataHandlers.GetSection += OnGetSection; GetDataHandlers.PlaceItemFrame += OnPlaceItemFrame; GetDataHandlers.GemLockToggle += OnGemLockToggle; GetDataHandlers.PlaceTileEntity += OnPlaceTileEntity; @@ -64,6 +65,28 @@ namespace TShockAPI GetDataHandlers.TileEdit += OnTileEdit; } + internal void OnGetSection(object sender, GetDataHandlers.GetSectionEventArgs args) + { + if (args.Player.RequestedSection) + { + args.Handled = true; + return; + } + args.Player.RequestedSection = true; + + if (String.IsNullOrEmpty(args.Player.Name)) + { + TShock.Utils.ForceKick(args.Player, "Blank name.", true); + args.Handled = true; + return; + } + + if (!args.Player.HasPermission(Permissions.ignorestackhackdetection)) + { + args.Player.IsDisabledForStackDetection = args.Player.HasHackedItemStacks(true); + } + } + /// Fired when an item frame is placed for anti-cheat detection. /// The object that triggered the event. /// The packet arguments that the event has. diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index 9162acd2..83bcb26d 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -58,6 +58,19 @@ namespace TShockAPI } } + /// + /// A custom HandledEventArgs that contains TShock's TSPlayer for the triggering uesr and the Terraria MP data stream. + /// Differentiated by GetDataHandlerArgs because it can be handled and responds to being handled. + /// + public class GetDataHandledEventArgs : HandledEventArgs + { + /// The TSPlayer that triggered the event. + public TSPlayer Player { get; set; } + + /// The raw MP packet data associated with the event. + public MemoryStream Data { get; set; } + } + public static class GetDataHandlers { private static Dictionary GetDataHandlerDelegates; @@ -1903,21 +1916,46 @@ namespace TShockAPI return true; } + /// The arguments to a GetSection packet. + public class GetSectionEventArgs : GetDataHandledEventArgs + { + /// The X position requested. Or -1 for spawn. + public int X { get; set; } + + /// The Y position requested. Or -1 for spawn. + public int Y { get; set; } + } + + /// The hook for a GetSection event. + public static HandlerList GetSection = new HandlerList(); + + /// Fires a GetSection event. + /// The TSPlayer that caused the GetSection. + /// The raw MP protocol data. + /// The x coordinate requested or -1 for spawn. + /// The y coordinate requested or -1 for spawn. + /// bool + private static bool OnGetSection(TSPlayer player, MemoryStream data, int x, int y) + { + if (GetSection == null) + return false; + + var args = new GetSectionEventArgs + { + Player = player, + Data = data, + X = x, + Y = y, + }; + + GetSection.Invoke(null, args); + return args.Handled; + } + private static bool HandleGetSection(GetDataHandlerArgs args) { - if (args.Player.RequestedSection) + if (OnGetSection(args.Player, args.Data, args.Data.ReadInt32(), args.Data.ReadInt32())) return true; - args.Player.RequestedSection = true; - if (String.IsNullOrEmpty(args.Player.Name)) - { - TShock.Utils.ForceKick(args.Player, "Blank name.", true); - return true; - } - - if (!args.Player.HasPermission(Permissions.ignorestackhackdetection)) - { - args.Player.IsDisabledForStackDetection = args.Player.HasHackedItemStacks(true); - } if (TShock.Utils.ActivePlayers() + 1 > TShock.Config.MaxSlots && !args.Player.HasPermission(Permissions.reservedslot)) From f93ffbc2e709b8f4defb041828ac2b1209ca3d24 Mon Sep 17 00:00:00 2001 From: Lucas Nicodemus Date: Thu, 21 Dec 2017 23:00:09 -0700 Subject: [PATCH 5/7] Implement named args in item stack hack check This addresses feedback from @QuiCM and @ijwu, who pointed out that C# allows you to specify the arguments that go into a call during invocation, which dramatically improves readability. --- TShockAPI/Bouncer.cs | 2 +- TShockAPI/TSPlayer.cs | 22 +++++++++++----------- TShockAPI/TShock.cs | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/TShockAPI/Bouncer.cs b/TShockAPI/Bouncer.cs index b0b6a427..3f14e977 100644 --- a/TShockAPI/Bouncer.cs +++ b/TShockAPI/Bouncer.cs @@ -83,7 +83,7 @@ namespace TShockAPI if (!args.Player.HasPermission(Permissions.ignorestackhackdetection)) { - args.Player.IsDisabledForStackDetection = args.Player.HasHackedItemStacks(true); + args.Player.IsDisabledForStackDetection = args.Player.HasHackedItemStacks(shouldWarnPlayer: true); } } diff --git a/TShockAPI/TSPlayer.cs b/TShockAPI/TSPlayer.cs index 23e187fb..89bf758c 100644 --- a/TShockAPI/TSPlayer.cs +++ b/TShockAPI/TSPlayer.cs @@ -308,9 +308,9 @@ namespace TShockAPI } /// Checks to see if a player has hacked item stacks in their inventory, and messages them as it checks. - /// If the check should send a message to the player with the results of the check. + /// If the check should send a message to the player with the results of the check. /// True if any stacks don't conform. - public bool HasHackedItemStacks(bool shouldSendMessage = false) + public bool HasHackedItemStacks(bool shouldWarnPlayer = false) { // Iterates through each inventory location a player has. // This section is sub divided into number ranges for what each range of slots corresponds to. @@ -340,7 +340,7 @@ namespace TShockAPI if (inventory[i].stack > item.maxStack || inventory[i].stack < 0) { check = true; - if (shouldSendMessage) + if (shouldWarnPlayer) { SendErrorMessage("Stack cheat detected. Remove item {0} ({1}) and then rejoin.", item.Name, inventory[i].stack); } @@ -360,7 +360,7 @@ namespace TShockAPI if (armor[index].stack > item.maxStack || armor[index].stack < 0) { check = true; - if (shouldSendMessage) + if (shouldWarnPlayer) { SendErrorMessage("Stack cheat detected. Remove armor {0} ({1}) and then rejoin.", item.Name, armor[index].stack); } @@ -380,7 +380,7 @@ namespace TShockAPI if (dye[index].stack > item.maxStack || dye[index].stack < 0) { check = true; - if (shouldSendMessage) + if (shouldWarnPlayer) { SendErrorMessage("Stack cheat detected. Remove dye {0} ({1}) and then rejoin.", item.Name, dye[index].stack); } @@ -400,7 +400,7 @@ namespace TShockAPI if (miscEquips[index].stack > item.maxStack || miscEquips[index].stack < 0) { check = true; - if (shouldSendMessage) + if (shouldWarnPlayer) { SendErrorMessage("Stack cheat detected. Remove item {0} ({1}) and then rejoin.", item.Name, miscEquips[index].stack); } @@ -420,7 +420,7 @@ namespace TShockAPI if (miscDyes[index].stack > item.maxStack || miscDyes[index].stack < 0) { check = true; - if (shouldSendMessage) + if (shouldWarnPlayer) { SendErrorMessage("Stack cheat detected. Remove item dye {0} ({1}) and then rejoin.", item.Name, miscDyes[index].stack); } @@ -441,7 +441,7 @@ namespace TShockAPI if (piggy[index].stack > item.maxStack || piggy[index].stack < 0) { check = true; - if (shouldSendMessage) + if (shouldWarnPlayer) { SendErrorMessage("Stack cheat detected. Remove piggy-bank item {0} ({1}) and then rejoin.", item.Name, piggy[index].stack); } @@ -462,7 +462,7 @@ namespace TShockAPI if (safe[index].stack > item.maxStack || safe[index].stack < 0) { check = true; - if (shouldSendMessage) + if (shouldWarnPlayer) { SendErrorMessage("Stack cheat detected. Remove safe item {0} ({1}) and then rejoin.", item.Name, safe[index].stack); } @@ -482,7 +482,7 @@ namespace TShockAPI if (trash.stack > item.maxStack) { check = true; - if (shouldSendMessage) + if (shouldWarnPlayer) { SendErrorMessage("Stack cheat detected. Remove trash item {0} ({1}) and then rejoin.", item.Name, trash.stack); } @@ -503,7 +503,7 @@ namespace TShockAPI if (forge[index].stack > item.maxStack || forge[index].stack < 0) { check = true; - if (shouldSendMessage) + if (shouldWarnPlayer) { SendErrorMessage("Stack cheat detected. Remove Defender's Forge item {0} ({1}) and then rejoin.", item.Name, forge[index].stack); } diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs index d466a22a..f9a61db8 100644 --- a/TShockAPI/TShock.cs +++ b/TShockAPI/TShock.cs @@ -1098,7 +1098,7 @@ namespace TShockAPI string check = "none"; if (!player.HasPermission(Permissions.ignorestackhackdetection)) { - player.IsDisabledForStackDetection = player.HasHackedItemStacks(true); + player.IsDisabledForStackDetection = player.HasHackedItemStacks(shouldWarnPlayer: true); } check = "none"; // Please don't remove this for the time being; without it, players wearing banned equipment will only get debuffed once From b93bab65d740ec63ebf123eb3a2110c8032f1b48 Mon Sep 17 00:00:00 2001 From: Lucas Nicodemus Date: Fri, 22 Dec 2017 10:21:49 -0700 Subject: [PATCH 6/7] Fix automatically disabling all players In rewriting Disable, I accidentally implemented logic that disabled all players for having banned armor. This is now fixed. --- TShockAPI/TShock.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs index dc9a4fc6..2c60ef2d 100644 --- a/TShockAPI/TShock.cs +++ b/TShockAPI/TShock.cs @@ -1081,7 +1081,6 @@ namespace TShockAPI } else if (!Main.ServerSideCharacter || (Main.ServerSideCharacter && player.IsLoggedIn)) { - string check = "none"; if (!player.HasPermission(Permissions.ignorestackhackdetection)) { player.IsDisabledForStackDetection = player.HasHackedItemStacks(shouldWarnPlayer: true); @@ -1140,7 +1139,8 @@ namespace TShockAPI break; } } - player.IsDisabledForBannedWearable = true; + if (check != "none") + player.IsDisabledForBannedWearable = true; if (player.IsBeingDisabled()) { From a2e6a069859798121f8d49f3c55c11dd29cf5770 Mon Sep 17 00:00:00 2001 From: Lucas Nicodemus Date: Fri, 22 Dec 2017 10:24:23 -0700 Subject: [PATCH 7/7] Fix local var check accidentally being removed --- TShockAPI/TShock.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TShockAPI/TShock.cs b/TShockAPI/TShock.cs index 2c60ef2d..ddb36d32 100644 --- a/TShockAPI/TShock.cs +++ b/TShockAPI/TShock.cs @@ -1085,7 +1085,7 @@ namespace TShockAPI { player.IsDisabledForStackDetection = player.HasHackedItemStacks(shouldWarnPlayer: true); } - check = "none"; + string check = "none"; // Please don't remove this for the time being; without it, players wearing banned equipment will only get debuffed once foreach (Item item in player.TPlayer.armor) {