Merge pull request #488 from CoderCow/patch-1

SSI Exploit Fix and Made /overidessi Actually Useful

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) <player name>");
+				args.Player.SendErrorMessage("Correct usage: /overridessi|/ossi <player name>");
 				return;
 			}
 
-			var players = TShock.Utils.FindPlayer(args.Parameters[0]);
+			string playerNameToMatch = string.Join(" ", args.Parameters);
-			if( players.Count < 1 )
+			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)
+
+			TSPlayer matchedPlayer = matchedPlayers[0];
+			if (matchedPlayer.IsLoggedIn)
 			{
-				if( players[0] != null && players[0].IsLoggedIn && !players[0].IgnoreActionsForClearingTrashCan)
+				args.Player.SendErrorMessage("Player \"{0}\" is already logged in.", matchedPlayer.Name);
-				{
+				return;
-					args.Player.SendSuccessMessage( players[0].Name + " has been exempted and updated.");
-					TShock.InventoryDB.InsertPlayerData(players[0]);
-				}
 			}
+			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)

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);
@@ -1210,6 +1219,13 @@ namespace TShockAPI
 			{
 				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;
 		}

TShockAPI/TSPlayer.cs

@@ -160,15 +160,21 @@ namespace TShockAPI
 		public string UserAccountName { get; set; }
 
         /// <summary>
-        /// 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.
         /// </summary>
-		public bool HasBeenSpammedWithBuildMessage;
+		public bool LoginFailsBySsi { get; set; }
 
         /// <summary>
         /// Whether the player is logged in or not.
         /// </summary>
 		public bool IsLoggedIn;
 
+        /// <summary>
+        /// Whether the player has sent their whole inventory to the server while connecting.
+        /// </summary>
+		public bool HasSentInventory { get; set; }
+
         /// <summary>
         /// The player's user id( from the db ).
         /// </summary>