From 25e584efe35c3c559862b8f3845b07b84c9f691a Mon Sep 17 00:00:00 2001 From: CoderCow Date: Mon, 1 Jul 2013 12:00:12 +0200 Subject: [PATCH] -Fixed an exploit where players could sneak items through SSI when DisableLoginBeforeJoin was true. -/overidessi will now actually work on players not logged in... --- TShockAPI/Commands.cs | 49 +++++++++++++++++++++++++----------- TShockAPI/GetDataHandlers.cs | 16 ++++++++++++ TShockAPI/TSPlayer.cs | 10 ++++++-- 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/TShockAPI/Commands.cs b/TShockAPI/Commands.cs index 3604a31c..6161055b 100644 --- a/TShockAPI/Commands.cs +++ b/TShockAPI/Commands.cs @@ -438,7 +438,6 @@ namespace TShockAPI } try { - // TODO: Is this needed? It seems to be an unreachable case if (user == null) { args.Player.SendErrorMessage("A user by that name does not exist."); @@ -457,11 +456,13 @@ namespace TShockAPI } else if (!TShock.CheckInventory(args.Player)) { + args.Player.LoginFailsBySsi = true; args.Player.SendErrorMessage("Login failed. Please fix the above errors then /login again."); args.Player.IgnoreActionsForClearingTrashCan = true; return; } } + args.Player.LoginFailsBySsi = false; if (group.HasPermission(Permissions.ignorestackhackdetection)) args.Player.IgnoreActionsForCheating = "none"; @@ -1180,29 +1181,49 @@ namespace TShockAPI public static void OverrideSSI( CommandArgs args ) { + if (!TShock.Config.ServerSideInventory) + { + args.Player.SendErrorMessage("Server Side Inventory is disabled."); + return; + } if( args.Parameters.Count < 1 ) { - args.Player.SendErrorMessage("Correct usage: /overridessi(/ossi) "); + args.Player.SendErrorMessage("Correct usage: /overridessi|/ossi "); return; } - var players = TShock.Utils.FindPlayer(args.Parameters[0]); - if( players.Count < 1 ) + string playerNameToMatch = string.Join(" ", args.Parameters); + var matchedPlayers = TShock.Utils.FindPlayer(playerNameToMatch); + if( matchedPlayers.Count < 1 ) { - args.Player.SendErrorMessage("No players match " + args.Parameters[0] + "!"); + args.Player.SendErrorMessage("No players matched \"{0}\".", playerNameToMatch); + return; } - else if( players.Count > 1 ) + else if( matchedPlayers.Count > 1 ) { - args.Player.SendErrorMessage( players.Count + " players matched " + args.Parameters[0] + "!"); + args.Player.SendErrorMessage("{0} players matched \"{1}\".", matchedPlayers.Count, playerNameToMatch); + return; } - else if (TShock.Config.ServerSideInventory) - { - if( players[0] != null && players[0].IsLoggedIn && !players[0].IgnoreActionsForClearingTrashCan) - { - args.Player.SendSuccessMessage( players[0].Name + " has been exempted and updated."); - TShock.InventoryDB.InsertPlayerData(players[0]); - } + + TSPlayer matchedPlayer = matchedPlayers[0]; + if (matchedPlayer.IsLoggedIn) + { + args.Player.SendErrorMessage("Player \"{0}\" is already logged in.", matchedPlayer.Name); + return; } + if (!matchedPlayer.LoginFailsBySsi) + { + args.Player.SendErrorMessage("Player \"{0}\" has to perform a /login attempt first.", matchedPlayer.Name); + return; + } + if (matchedPlayer.IgnoreActionsForClearingTrashCan) + { + args.Player.SendErrorMessage("Player \"{0}\" has to reconnect first.", matchedPlayer.Name); + return; + } + + TShock.InventoryDB.InsertPlayerData(matchedPlayer); + args.Player.SendSuccessMessage("SSI of player \"{0}\" has been overriden.", matchedPlayer.Name); } private static void ForceXmas(CommandArgs args) diff --git a/TShockAPI/GetDataHandlers.cs b/TShockAPI/GetDataHandlers.cs index 46f112b8..9b37250d 100644 --- a/TShockAPI/GetDataHandlers.cs +++ b/TShockAPI/GetDataHandlers.cs @@ -1189,6 +1189,14 @@ namespace TShockAPI byte prefix = args.Data.ReadInt8(); short type = args.Data.ReadInt16(); + // Players send a slot update packet for each inventory slot right after they've joined. + bool bypassTrashCanCheck = false; + if (plr == args.Player.Index && !args.Player.HasSentInventory && slot == NetItem.maxNetInventory) + { + args.Player.HasSentInventory = true; + bypassTrashCanCheck = true; + } + if (OnPlayerSlot(plr, slot, stack, prefix, type)) return true; @@ -1202,6 +1210,7 @@ namespace TShockAPI return true; } + // Garabage? Or will it cause some internal initialization or whatever? var item = new Item(); item.netDefaults(type); item.Prefix(prefix); @@ -1209,6 +1218,13 @@ namespace TShockAPI if (args.Player.IsLoggedIn) { args.Player.PlayerData.StoreSlot(slot, type, prefix, stack); + } + else if ( + TShock.Config.ServerSideInventory && TShock.Config.DisableLoginBeforeJoin && !bypassTrashCanCheck && + args.Player.HasSentInventory && !args.Player.Group.HasPermission(Permissions.bypassinventorychecks) + ) { + // The player might have moved an item to their trash can before they performed a single login attempt yet. + args.Player.IgnoreActionsForClearingTrashCan = true; } return false; diff --git a/TShockAPI/TSPlayer.cs b/TShockAPI/TSPlayer.cs index b0ec9f4c..d9182329 100644 --- a/TShockAPI/TSPlayer.cs +++ b/TShockAPI/TSPlayer.cs @@ -160,15 +160,21 @@ namespace TShockAPI public string UserAccountName { get; set; } /// - /// Unused can be removed. + /// Whether the player performed a valid login attempt (i.e. entered valid user name and password) but is still blocked + /// from logging in because of SSI. /// - public bool HasBeenSpammedWithBuildMessage; + public bool LoginFailsBySsi { get; set; } /// /// Whether the player is logged in or not. /// public bool IsLoggedIn; + /// + /// Whether the player has sent their whole inventory to the server while connecting. + /// + public bool HasSentInventory { get; set; } + /// /// The player's user id( from the db ). ///