From 50ee25e851ab4be00bb49d9968bf7a8f0f431238 Mon Sep 17 00:00:00 2001 From: Christopher Andrews Date: Wed, 9 Apr 2014 16:36:40 +1000 Subject: [PATCH] Defend against specially-crafted vulnerable messages. --- KMPServer/Server.cs | 129 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 107 insertions(+), 22 deletions(-) diff --git a/KMPServer/Server.cs b/KMPServer/Server.cs index 054bc7d..6b3f14d 100644 --- a/KMPServer/Server.cs +++ b/KMPServer/Server.cs @@ -1923,7 +1923,6 @@ public void queueClientMessage(Client cl, KMPCommon.ClientMessageID id, byte[] d clientMessageQueue.Enqueue(message); } - private KMPCommon.ClientMessageID[] AllowNullDataMessages = { KMPCommon.ClientMessageID.SCREEN_WATCH_PLAYER, KMPCommon.ClientMessageID.CONNECTION_END, KMPCommon.ClientMessageID.ACTIVITY_UPDATE_IN_FLIGHT, KMPCommon.ClientMessageID.ACTIVITY_UPDATE_IN_GAME, KMPCommon.ClientMessageID.SYNC_TIME }; private KMPCommon.ClientMessageID[] AllowClientNotReadyMessages = { KMPCommon.ClientMessageID.HANDSHAKE, KMPCommon.ClientMessageID.TEXT_MESSAGE, KMPCommon.ClientMessageID.SCREENSHOT_SHARE, KMPCommon.ClientMessageID.CONNECTION_END, KMPCommon.ClientMessageID.ACTIVITY_UPDATE_IN_FLIGHT, KMPCommon.ClientMessageID.ACTIVITY_UPDATE_IN_GAME, KMPCommon.ClientMessageID.PING, KMPCommon.ClientMessageID.UDP_PROBE, KMPCommon.ClientMessageID.WARPING, KMPCommon.ClientMessageID.SSYNC, KMPCommon.ClientMessageID.SYNC_TIME }; public void handleMessage(Client cl, KMPCommon.ClientMessageID id, byte[] data) @@ -1933,7 +1932,6 @@ public void handleMessage(Client cl, KMPCommon.ClientMessageID id, byte[] data) if (!cl.isValid) { return; } - if (!AllowNullDataMessages.Contains(id) && data == null) { return; } if (!AllowClientNotReadyMessages.Contains(id) && !cl.isReady) { return; } try @@ -2014,6 +2012,13 @@ public void handleMessage(Client cl, KMPCommon.ClientMessageID id, byte[] data) private void HandleSSync(Client cl, byte[] data) { + //Message format: subspace request(4) + if (data != null ? data.Length != 4 : true) + { + Log.Debug("Malformed SSYNC message from player " + cl.playerID); + return; + } + int subspaceID = KMPCommon.intFromBytes(data, 0); if (subspaceID == -1) { @@ -2029,6 +2034,12 @@ record => subspaceID = record.GetInt32(0)); private void HandleTimeSync(Client cl, byte[] data) { //Message format: clientsendtick(8), serverreceivetick(8), serversendtick(8). The server send tick gets added during actual sending. + if (data != null ? data.Length != 4 : true) + { + Log.Debug("Malformed TIME_SYNC message from player " + cl.playerID); + return; + } + byte[] message_bytes = buildMessageArray(KMPCommon.ServerMessageID.SYNC_TIME, data); //This has already been rewritten in the queueClientMessage. cl.queueOutgoingMessage(message_bytes); //This is still re-written during the actual send. Log.Debug("{0} time sync request", cl.username); @@ -2036,6 +2047,13 @@ private void HandleTimeSync(Client cl, byte[] data) private void HandleWarping(Client cl, byte[] data) { + //Message format: rate(4), new subspace tick(8) + if (data != null ? data.Length != 12 : true) + { + Log.Debug("Malformed WARPING message from player " + cl.playerID); + return; + } + float rate = BitConverter.ToSingle(data, 0); double newsubspacetick = BitConverter.ToDouble(data, 4); if (cl.warping) @@ -2056,7 +2074,6 @@ private void HandleWarping(Client cl, byte[] data) subSpaceMasterSpeed.Add(cl.currentSubspaceID, 1f); cl.warping = false; sendSubspace(cl, true, true); - cl.lastTick = newsubspacetick; Log.Activity("{0} set to new subspace {1}", cl.username, newSubspace); } } @@ -2073,6 +2090,13 @@ private void HandleWarping(Client cl, byte[] data) private void HandleUDPProbe(Client cl, byte[] data) { + //Message format: current tick(8), average clock skew(4) + if (data != null ? data.Length != 12 : true) + { + Log.Debug("Malformed UDP_PROBE message from player " + cl.playerID); + return; + } + double incomingTick = BitConverter.ToDouble(data, 0); double lastSubspaceTick = incomingTick; @@ -2118,13 +2142,24 @@ private void HandleActivityUpdateInFlight(Client cl) private void HandleShareCraftFile(Client cl, byte[] data, UnicodeEncoding encoder) { - if (!(data.Length > 8 && (data.Length - 8) <= KMPCommon.MAX_CRAFT_FILE_BYTES)) { return; } + //Message format: type(4), name length(4), craft name(variable), craft data(varible) + if (data != null ? (data.Length < 8 || data.Length > KMPCommon.MAX_CRAFT_FILE_BYTES) : true) + { + Log.Debug("Malformed SHARE_CRAFT_FILE message from player " + cl.playerID + ", check 1"); + return; + } //Read craft name length KMPCommon.CraftType craft_type = (KMPCommon.CraftType)KMPCommon.intFromBytes(data, 0); int craft_name_length = KMPCommon.intFromBytes(data, 4); - if (craft_name_length < data.Length - 8) + + //Check name length data + if (data != null ? data.Length < (8 + craft_name_length) : true) { + Log.Debug("Malformed SHARE_CRAFT_FILE message from player " + cl.playerID + ", check 2"); + return; + } + //Read craft name String craft_name = encoder.GetString(data, 8, craft_name_length); @@ -2166,21 +2201,28 @@ private void HandleShareCraftFile(Client cl, byte[] data, UnicodeEncoding encode sb.Append(cl.username); sb.Append(" to get it."); sendTextMessageToAll(sb.ToString()); - } } private void HandleConnectionEnd(Client cl, byte[] data, UnicodeEncoding encoder) { + //Message format: reason(variable, optional) String message = String.Empty; if (data != null) + { message = encoder.GetString(data, 0, data.Length); //Decode the message + } markClientForDisconnect(cl, message); //Disconnect the client } private void HandleScreenshotShare(Client cl, byte[] data) { - if (data.Length > settings.screenshotSettings.maxNumBytes) { return; } + //Message format: screenshot(variable) + if (data != null ? data.Length > settings.screenshotSettings.maxNumBytes : true) + { + Log.Debug("Malformed SCREENSHOT_SHARE message from player " + cl.playerID); + return; + } //Set the screenshot for the player lock (cl.screenshotLock) @@ -2204,10 +2246,14 @@ private void HandleScreenshotShare(Client cl, byte[] data) private void HandleScreenWatchPlayer(Client cl, byte[] data, UnicodeEncoding encoder) { - String watch_name = String.Empty; + //Message format: name (variable) + if (data == null) + { + Log.Debug("Malformed SCREEN_WATCH_PLAYER message from player " + cl.playerID); + return; + } - if (data != null) - watch_name = encoder.GetString(data); + String watch_name = encoder.GetString(data); bool watch_name_changed = false; @@ -2221,8 +2267,7 @@ private void HandleScreenWatchPlayer(Client cl, byte[] data, UnicodeEncoding enc } } - if (watch_name_changed && watch_name.Length > 0 - && watch_name != cl.username) + if (watch_name_changed && watch_name.Length > 0 && watch_name != cl.username) { //Try to find the player the client is watching and send that player's current screenshot Client watch_client = getClientByName(watch_name); @@ -2233,15 +2278,21 @@ private void HandleScreenWatchPlayer(Client cl, byte[] data, UnicodeEncoding enc { screenshot = watch_client.screenshot; } - if (screenshot != null) + { sendScreenshot(cl, watch_client.screenshot); + } } } } private void HandlePluginUpdate(Client cl, KMPCommon.ClientMessageID id, byte[] data) { + if (data == null) + { + Log.Debug("Malformed UPDATE message from player " + cl.playerID); + return; + } if (cl.isReady) { sendPluginUpdateToAll(data, id == KMPCommon.ClientMessageID.SECONDARY_PLUGIN_UPDATE, cl); @@ -2250,6 +2301,11 @@ private void HandlePluginUpdate(Client cl, KMPCommon.ClientMessageID id, byte[] private void HandleScenarioUpdate(Client cl, byte[] data) { + if (data == null) + { + Log.Debug("Malformed SCENARIO_UPDATE message from player " + cl.playerID + ", check 1"); + return; + } if (cl.isReady) { var scenario_update = ByteArrayToObject(data); @@ -2258,39 +2314,64 @@ private void HandleScenarioUpdate(Client cl, byte[] data) { Log.Debug("Received scenario update '{1}' from {0}", cl.username, scenario_update.name); object result = Database.ExecuteScalar("SELECT ID FROM kmpScenarios WHERE PlayerID = @playerID AND Name = @name;", - "playerID", cl.playerID, - "name", scenario_update.name); + "playerID", cl.playerID, + "name", scenario_update.name); if (result == null) { Database.ExecuteNonQuery("INSERT INTO kmpScenarios (PlayerID, Name, Tick, UpdateMessage)" + " VALUES (@playerID, @name, @tick, @updateMessage);", - "playerID", cl.playerID, - "name", scenario_update.name, - "tick", scenario_update.tick.ToString("0.0").Replace(",", "."), - "updateMessage", data); + "playerID", cl.playerID, + "name", scenario_update.name, + "tick", scenario_update.tick.ToString("0.0").Replace(",", "."), + "updateMessage", data); } else { Database.ExecuteNonQuery("UPDATE kmpScenarios SET Tick = @tick, UpdateMessage = @updateMessage WHERE ID = @id", - "id", Convert.ToInt32(result), - "tick", scenario_update.tick.ToString("0.0").Replace(",", "."), - "updateMessage", data); + "id", Convert.ToInt32(result), + "tick", scenario_update.tick.ToString("0.0").Replace(",", "."), + "updateMessage", data); } } + else + { + Log.Debug("Malformed SCENARIO_UPDATE message from player " + cl.playerID + ", check 2"); + } } } private void HandleHandshake(Client cl, byte[] data, UnicodeEncoding encoder) { + //Message format: + if (data != null ? data.Length < 4 : true) + { + Log.Debug("Malformed HANDSHAKE message from player " + cl.playerID + ", check 1"); + return; + } + StringBuilder sb = new StringBuilder(); //Read username Int32 username_length = KMPCommon.intFromBytes(data, 0); + + if (data != null ? data.Length < (4 + username_length + 4) : true) + { + Log.Debug("Malformed HANDSHAKE message from player " + cl.playerID + ", check 2"); + return; + } + String username = encoder.GetString(data, 4, username_length); Guid guid = Guid.Empty; Int32 guid_length = KMPCommon.intFromBytes(data, 4 + username_length); int offset = 4 + username_length + 4; + + if (data != null ? data.Length < (offset + guid_length) : true) + { + Log.Debug("Malformed HANDSHAKE message from player " + cl.playerID + ", check 3"); + return; + } + try { guid = new Guid(encoder.GetString(data, offset, guid_length)); @@ -2299,8 +2380,10 @@ private void HandleHandshake(Client cl, byte[] data, UnicodeEncoding encoder) { markClientForDisconnect(cl, "You're authentication token is not valid."); Log.Info("Rejected client due to invalid guid: {0}", encoder.GetString(data, offset, guid_length)); + return; } offset = 4 + username_length + 4 + guid_length; + String version = encoder.GetString(data, offset, data.Length - offset); String username_lower = username.ToLower(); @@ -2332,7 +2415,9 @@ private void HandleHandshake(Client cl, byte[] data, UnicodeEncoding encoder) } if (!accepted) + { return; + } //Check if this player is new to universe int name_taken = Convert.ToInt32(