From 6f444488a5ed2ed0283b128304ba74e7ed65fc8f Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 7 Jun 2024 18:08:07 -0700 Subject: [PATCH] fix: Fixes crashing due to bad packet assumptions. (#1829) ### Summary - Fixes various exploits that can crash the shard when the client misbehaves. - Clients will now be disconnected if they send packets that are marked as out of game only (new flag), while they are in-game. > [!Note] > **Developer Note** > Added an `OutOfGameOnly` which should be used to flag packets as only available out of the game. > This is the opposite of, yet not the converse to `InGameOnly`. --- Projects/Server/Network/NetState/NetState.cs | 11 +++++- Projects/Server/Network/PacketHandler.cs | 17 +++++++-- .../Server/Network/Packets/IncomingPackets.cs | 17 +++++++-- .../UOContent/Assistants/AssistantProtocol.cs | 8 +++- .../Network/ContainerGridPacketHandler.cs | 5 +-- .../UOContent/Network/FreeshardProtocol.cs | 9 ++++- .../Network/Packets/IncomingAccountPackets.cs | 23 ++++++----- .../Packets/IncomingExtendedCommandPackets.cs | 38 +++++++++++++------ .../Network/Packets/IncomingItemPackets.cs | 2 +- .../UOContent/Network/ProtocolExtensions.cs | 29 +++++++++----- 10 files changed, 113 insertions(+), 46 deletions(-) diff --git a/Projects/Server/Network/NetState/NetState.cs b/Projects/Server/Network/NetState/NetState.cs index 5391b10d9..ef1080885 100755 --- a/Projects/Server/Network/NetState/NetState.cs +++ b/Projects/Server/Network/NetState/NetState.cs @@ -806,20 +806,27 @@ public partial class NetState : IComparable, IValueLinkListNode onReceive) + public PacketHandler( + int packetID, delegate* onReceive, + int length = 0, bool inGameOnly = false, bool outGameOnly = false + ) : this(packetID, length, inGameOnly, outGameOnly, onReceive) + { + + } + + public PacketHandler(int packetID, int length, bool inGameOnly, bool outGameOnly, delegate* onReceive) { _length = length; PacketID = packetID; - Ingame = ingame; + InGameOnly = inGameOnly; + OutOfGameOnly = outGameOnly; OnReceive = onReceive; } @@ -37,5 +46,7 @@ public unsafe class PacketHandler public delegate* ThrottleCallback { get; set; } - public bool Ingame { get; } + public bool InGameOnly { get; } + + public bool OutOfGameOnly { get; } } diff --git a/Projects/Server/Network/Packets/IncomingPackets.cs b/Projects/Server/Network/Packets/IncomingPackets.cs index a144b2d55..d257e8b5b 100644 --- a/Projects/Server/Network/Packets/IncomingPackets.cs +++ b/Projects/Server/Network/Packets/IncomingPackets.cs @@ -25,9 +25,20 @@ public static class IncomingPackets public static PacketHandler[] Handlers { get; } = new PacketHandler[0x100]; [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static unsafe void Register(int packetID, int length, bool ingame, - delegate* onReceive) => - Register(new PacketHandler(packetID, length, ingame, onReceive)); + public static unsafe void Register( + int packetID, delegate* onReceive, int length = 0, bool ingameOnly = false, + bool outgameOnly = false + ) => Register(packetID, length, ingameOnly, outgameOnly, onReceive); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static unsafe void Register( + int packetID, int length, bool ingame, delegate* onReceive + ) => Register(packetID, length, ingame, false, onReceive); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static unsafe void Register( + int packetID, int length, bool ingame, bool outgame, delegate* onReceive + ) => Register(new PacketHandler(packetID, length, ingame, outgame, onReceive)); public static void Register(PacketHandler packetHandler) { diff --git a/Projects/UOContent/Assistants/AssistantProtocol.cs b/Projects/UOContent/Assistants/AssistantProtocol.cs index 3f9ebd233..52a3c8616 100644 --- a/Projects/UOContent/Assistants/AssistantProtocol.cs +++ b/Projects/UOContent/Assistants/AssistantProtocol.cs @@ -1,4 +1,5 @@ using System.Buffers; +using System.Runtime.CompilerServices; namespace Server.Network; @@ -12,8 +13,13 @@ public static class AssistantProtocol _handlers = ProtocolExtensions.Register(new AssistantsProtocolInfo()); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static unsafe void Register(int cmd, bool ingame, delegate* onReceive) => - _handlers[cmd] = new PacketHandler(cmd, 0, ingame, onReceive); + Register(cmd, ingame, false, onReceive); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static unsafe void Register(int cmd, bool ingame, bool outgame, delegate* onReceive) => + _handlers[cmd] = new PacketHandler(cmd, onReceive, inGameOnly: ingame, outGameOnly: outgame); private struct AssistantsProtocolInfo : IProtocolExtensionsInfo { diff --git a/Projects/UOContent/Network/ContainerGridPacketHandler.cs b/Projects/UOContent/Network/ContainerGridPacketHandler.cs index 8d3d016eb..20f37d695 100644 --- a/Projects/UOContent/Network/ContainerGridPacketHandler.cs +++ b/Projects/UOContent/Network/ContainerGridPacketHandler.cs @@ -19,9 +19,8 @@ namespace Server.Network; public unsafe class ContainerGridPacketHandler : PacketHandler { - public ContainerGridPacketHandler(int packetID, int length, bool ingame, - delegate* onReceive) - : base(packetID, length, ingame, onReceive) + public ContainerGridPacketHandler(int packetID, int length, delegate* onReceive) + : base(packetID, length, true, false, onReceive) { } diff --git a/Projects/UOContent/Network/FreeshardProtocol.cs b/Projects/UOContent/Network/FreeshardProtocol.cs index 0c614fba6..ada158275 100644 --- a/Projects/UOContent/Network/FreeshardProtocol.cs +++ b/Projects/UOContent/Network/FreeshardProtocol.cs @@ -14,6 +14,7 @@ *************************************************************************/ using System.Buffers; +using System.Runtime.CompilerServices; namespace Server.Network { @@ -27,8 +28,14 @@ namespace Server.Network _handlers = ProtocolExtensions.Register(new FreeshardProtocolInfo()); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static unsafe void Register(int cmd, bool ingame, delegate* onReceive) => - _handlers[cmd] = new PacketHandler(cmd, 0, ingame, onReceive); + Register(cmd, ingame, false, onReceive); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static unsafe void Register( + int cmd, bool ingame, bool outgame, delegate* onReceive + ) => _handlers[cmd] = new PacketHandler(cmd, onReceive, inGameOnly: ingame, outGameOnly: outgame); private struct FreeshardProtocolInfo : IProtocolExtensionsInfo { diff --git a/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs b/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs index a6a87ae80..eb8a89e83 100644 --- a/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs +++ b/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs @@ -57,18 +57,17 @@ public static class IncomingAccountPackets public static unsafe void Configure() { - IncomingPackets.Register(0x00, 104, false, &CreateCharacter); - IncomingPackets.Register(0x5D, 73, false, &PlayCharacter); - IncomingPackets.Register(0x80, 62, false, &AccountLogin); - IncomingPackets.Register(0x83, 39, false, &DeleteCharacter); - IncomingPackets.Register(0x91, 65, false, &GameLogin); - IncomingPackets.Register(0xA0, 3, false, &PlayServer); - IncomingPackets.Register(0xBB, 9, false, &AccountID); - IncomingPackets.Register(0xBD, 0, false, &ClientVersion); - IncomingPackets.Register(0xCF, 0, false, &AccountLogin); - IncomingPackets.Register(0xE1, 0, false, &ClientType); - IncomingPackets.Register(0xEF, 21, false, &LoginServerSeed); - IncomingPackets.Register(0xF8, 106, false, &CreateCharacter); + IncomingPackets.Register(0x00, &CreateCharacter, 104, outgameOnly: true); + IncomingPackets.Register(0x5D, &PlayCharacter, 73, outgameOnly: true); + IncomingPackets.Register(0x80, &AccountLogin, 62, outgameOnly: true); + IncomingPackets.Register(0x83, &DeleteCharacter, 39, outgameOnly: true); + IncomingPackets.Register(0x91, &GameLogin, 65, outgameOnly: true); + IncomingPackets.Register(0xA0, &PlayServer, 3, outgameOnly: true); + IncomingPackets.Register(0xBD, &ClientVersion); + IncomingPackets.Register(0xCF, &AccountLogin, outgameOnly: true); + IncomingPackets.Register(0xE1, &ClientType); + IncomingPackets.Register(0xEF, &LoginServerSeed, 21, outgameOnly: true); + IncomingPackets.Register(0xF8, &CreateCharacter, 106, outgameOnly: true); } public static void CreateCharacter(NetState state, SpanReader reader) diff --git a/Projects/UOContent/Network/Packets/IncomingExtendedCommandPackets.cs b/Projects/UOContent/Network/Packets/IncomingExtendedCommandPackets.cs index ec7c73f3e..4e8afed42 100644 --- a/Projects/UOContent/Network/Packets/IncomingExtendedCommandPackets.cs +++ b/Projects/UOContent/Network/Packets/IncomingExtendedCommandPackets.cs @@ -14,6 +14,7 @@ *************************************************************************/ using System.Buffers; +using System.Runtime.CompilerServices; using Server.ContextMenus; using Server.Items; using Server.Mobiles; @@ -70,12 +71,18 @@ public static class IncomingExtendedCommandPackets { } - public static unsafe void RegisterExtended(int packetID, bool ingame, - delegate* onReceive) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static unsafe void RegisterExtended( + int packetID, bool ingame, delegate* onReceive + ) => RegisterExtended(packetID, ingame, false, onReceive); + + public static unsafe void RegisterExtended( + int packetID, bool ingame, bool outgame, delegate* onReceive + ) { if (packetID is >= 0 and < 0x100) { - _extendedHandlers[packetID] = new PacketHandler(packetID, 0, ingame, onReceive); + _extendedHandlers[packetID] = new PacketHandler(packetID, onReceive, inGameOnly: ingame, outGameOnly: outgame); } } @@ -102,21 +109,30 @@ public static class IncomingExtendedCommandPackets return; } - if (ph.Ingame && state.Mobile?.Deleted != false) + var from = state.Mobile; + + if (ph.InGameOnly) { - if (state.Mobile == null) + if (from == null) { - state.LogInfo( - $"Sent in-game packet (0xBFx{packetId:X2}) before having been attached to a mobile" - ); + state.Disconnect($"Received packet 0x{packetId:X2} before having been attached to a mobile."); + return; } - state.Disconnect($"Sent in-game packet(0xBFx{packetId:X2}) but mobile is deleted."); + if (from.Deleted) + { + state.Disconnect($"Received packet 0x{packetId:X2} after having been attached to a deleted mobile."); + return; + } } - else + + if (ph.OutOfGameOnly && from?.Deleted == false) { - ph.OnReceive(state, reader); + state.Disconnect($"Received packet 0x{packetId:X2} after having been attached to a mobile."); + return; } + + ph.OnReceive(state, reader); } public static void ScreenSize(NetState state, SpanReader reader) diff --git a/Projects/UOContent/Network/Packets/IncomingItemPackets.cs b/Projects/UOContent/Network/Packets/IncomingItemPackets.cs index e770b00c1..f2ad7fd8f 100644 --- a/Projects/UOContent/Network/Packets/IncomingItemPackets.cs +++ b/Projects/UOContent/Network/Packets/IncomingItemPackets.cs @@ -26,7 +26,7 @@ public static class IncomingItemPackets public static unsafe void Configure() { IncomingPackets.Register(0x07, 7, true, &LiftReq); - IncomingPackets.Register(new ContainerGridPacketHandler(0x08, 14, true, &DropReq)); + IncomingPackets.Register(new ContainerGridPacketHandler(0x08, 14, &DropReq)); IncomingPackets.Register(0x13, 10, true, &EquipReq); IncomingPackets.Register(0xEC, 0, false, &EquipMacro); IncomingPackets.Register(0xED, 0, false, &UnequipMacro); diff --git a/Projects/UOContent/Network/ProtocolExtensions.cs b/Projects/UOContent/Network/ProtocolExtensions.cs index eb86339d7..abbec8b46 100644 --- a/Projects/UOContent/Network/ProtocolExtensions.cs +++ b/Projects/UOContent/Network/ProtocolExtensions.cs @@ -46,19 +46,30 @@ namespace Server.Network return; } - if (ph.Ingame && state.Mobile == null) + var from = state.Mobile; + + if (ph.InGameOnly) { - state.LogInfo($"Sent in-game packet (0x{packetId:X2}x{cmd:X2}) before having been attached to a mobile"); - state.Disconnect("Sent in-game packet before being attached to a mobile."); + if (from == null) + { + state.Disconnect($"Received packet 0x{packetId:X2}x{cmd:X2} before having been attached to a mobile."); + return; + } + + if (from.Deleted) + { + state.Disconnect($"Received packet 0x{packetId:X2}x{cmd:X2} after having been attached to a deleted mobile."); + return; + } } - else if (ph.Ingame && state.Mobile.Deleted) + + if (ph.OutOfGameOnly && from?.Deleted == false) { - state.Disconnect(string.Empty); - } - else - { - ph.OnReceive(state, reader); + state.Disconnect($"Received packet 0x{packetId:X2}x{cmd:X2} after having been attached to a mobile."); + return; } + + ph.OnReceive(state, reader); } } }