From 62e5a8761013a827b611dc5e8bb47b597f094540 Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:16:59 -0500 Subject: [PATCH 01/10] Upgrade MBNCSUtil submodule to net10.0 Co-Authored-By: Claude Opus 4.6 --- vendor/MBNCSUtil | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/MBNCSUtil b/vendor/MBNCSUtil index 693e1526..a36d853e 160000 --- a/vendor/MBNCSUtil +++ b/vendor/MBNCSUtil @@ -1 +1 @@ -Subproject commit 693e1526910b2947b5725401593dd97bfffe6f78 +Subproject commit a36d853e015011a5ff14fc1ef3e699299c37c811 From 04977d03479f999c114ee392f5dc19b56134c173 Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:17:19 -0500 Subject: [PATCH 02/10] Upgrade Atlasd to net10.0, remove obsolete UseOnlyOverlappedIO Socket.UseOnlyOverlappedIO was removed in .NET 5+. Overlapped I/O is the default behavior for async socket operations on modern .NET. Co-Authored-By: Claude Opus 4.6 --- src/Atlasd/Atlasd.csproj | 2 +- src/Atlasd/Battlenet/Protocols/HTTP/HttpListener.cs | 1 - src/Atlasd/Battlenet/ServerSocket.cs | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Atlasd/Atlasd.csproj b/src/Atlasd/Atlasd.csproj index 89a610ef..7c845d56 100644 --- a/src/Atlasd/Atlasd.csproj +++ b/src/Atlasd/Atlasd.csproj @@ -2,7 +2,7 @@ Exe - netcoreapp3.1 + net10.0 Atlas is cross-platform software that emulates Classic Battle.net in a compatible model for Diablo, StarCraft, and WarCraft. © 2020-2021 Carl Bennett <carl@carlbennett.me> BNETDocs diff --git a/src/Atlasd/Battlenet/Protocols/HTTP/HttpListener.cs b/src/Atlasd/Battlenet/Protocols/HTTP/HttpListener.cs index 0c8eadf9..371790bb 100644 --- a/src/Atlasd/Battlenet/Protocols/HTTP/HttpListener.cs +++ b/src/Atlasd/Battlenet/Protocols/HTTP/HttpListener.cs @@ -64,7 +64,6 @@ public void Start() /* part of IListener */ { ExclusiveAddressUse = true, NoDelay = Daemon.Common.TcpNoDelay, - UseOnlyOverlappedIO = true, }; Socket.Bind(LocalEndPoint); Socket.Listen(-1); diff --git a/src/Atlasd/Battlenet/ServerSocket.cs b/src/Atlasd/Battlenet/ServerSocket.cs index 6d4a16ca..f83b31d7 100644 --- a/src/Atlasd/Battlenet/ServerSocket.cs +++ b/src/Atlasd/Battlenet/ServerSocket.cs @@ -82,7 +82,6 @@ public void SetLocalEndPoint(IPEndPoint localEndPoint) { ExclusiveAddressUse = true, NoDelay = Daemon.Common.TcpNoDelay, - UseOnlyOverlappedIO = true, }; } From b0635c901c27bb3f51dbe4e25c8849ff8a83e758 Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:17:31 -0500 Subject: [PATCH 03/10] Update CI workflow for .NET 10.0 Co-Authored-By: Claude Opus 4.6 --- .github/workflows/dotnet-core.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/dotnet-core.yml b/.github/workflows/dotnet-core.yml index a1ad74c9..0bf0ab21 100644 --- a/.github/workflows/dotnet-core.yml +++ b/.github/workflows/dotnet-core.yml @@ -1,4 +1,4 @@ -name: .NET Core +name: .NET on: push: @@ -15,13 +15,13 @@ jobs: working-directory: ./src/Atlasd steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Checkout submodules run: git submodule update --init --recursive - - name: Setup .NET Core - uses: actions/setup-dotnet@v1 + - name: Setup .NET + uses: actions/setup-dotnet@v4 with: - dotnet-version: 3.1.301 + dotnet-version: '10.0.x' - name: Install dependencies run: dotnet restore working-directory: ${{env.working-directory}} From 6a34c2a85b55cf72a5bb0b0fd3d7568573ef202b Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:17:44 -0500 Subject: [PATCH 04/10] Fix Friend.Sync deadlock via consistent lock ordering Friend.Sync() locked source then target. If two users interacted simultaneously with swapped source/target, ABBA deadlock occurred. Fix by always locking the object with the lower identity hash first. Co-Authored-By: Claude Opus 4.6 --- src/Atlasd/Battlenet/Friend.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Atlasd/Battlenet/Friend.cs b/src/Atlasd/Battlenet/Friend.cs index 2ecea3fb..75fd79fa 100644 --- a/src/Atlasd/Battlenet/Friend.cs +++ b/src/Atlasd/Battlenet/Friend.cs @@ -60,9 +60,20 @@ public void Sync(GameState source) return; } - lock (source) + // Enforce consistent lock ordering to prevent ABBA deadlock. + // Always lock the object with the lower RuntimeHelpers hash first. + var first = source; + var second = target; + if (System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(source) > + System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(target)) { - lock (target) + first = target; + second = source; + } + + lock (first) + { + lock (second) { var admin = source.HasAdmin(); var mutual = false; From ed2b31591a3e971c8ced07bf42756da3c840085c Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:17:53 -0500 Subject: [PATCH 05/10] Fix ReceiveBuffer race condition with dedicated lock object ProcessReceive() locked on ReceiveBuffer (a byte[]) then reassigned it inside the lock. Subsequent calls locked on a different object, breaking mutual exclusion entirely. Replace with a dedicated readonly _receiveLock object used consistently for all buffer access. Co-Authored-By: Claude Opus 4.6 --- src/Atlasd/Battlenet/ClientState.cs | 351 ++++++++++++++-------------- 1 file changed, 182 insertions(+), 169 deletions(-) diff --git a/src/Atlasd/Battlenet/ClientState.cs b/src/Atlasd/Battlenet/ClientState.cs index 227c34f7..f23b8ac1 100644 --- a/src/Atlasd/Battlenet/ClientState.cs +++ b/src/Atlasd/Battlenet/ClientState.cs @@ -19,7 +19,7 @@ class ClientState { public BNFTPState BNFTPState; public bool Connected { get => Socket != null && Socket.Connected; } - public bool IsClosing { get; private set; } = false; + public volatile bool IsClosing = false; public GameState GameState { get; private set; } public ProtocolType ProtocolType { get; private set; } @@ -29,6 +29,7 @@ class ClientState protected byte[] ReceiveBuffer = new byte[0]; protected byte[] SendBuffer = new byte[0]; + private readonly object _receiveLock = new object(); protected Frame BattlenetGameFrame = new Frame(); @@ -160,7 +161,7 @@ public void ProcessReceive(SocketAsyncEventArgs e) } // Append received data to previously received data - lock (ReceiveBuffer) + lock (_receiveLock) { var newBuffer = new byte[ReceiveBuffer.Length + e.BytesTransferred]; Buffer.BlockCopy(ReceiveBuffer, 0, newBuffer, 0, ReceiveBuffer.Length); @@ -216,8 +217,11 @@ protected void ReceiveProtocolType(SocketAsyncEventArgs e) { if (ProtocolType != null) return; - ProtocolType = new ProtocolType((ProtocolType.Types)ReceiveBuffer[0]); - ReceiveBuffer = ReceiveBuffer[1..]; + lock (_receiveLock) + { + ProtocolType = new ProtocolType((ProtocolType.Types)ReceiveBuffer[0]); + ReceiveBuffer = ReceiveBuffer[1..]; + } if (ProtocolType.IsGame() || ProtocolType.IsChat()) { @@ -261,229 +265,238 @@ protected void ReceiveProtocol(SocketAsyncEventArgs e) protected void ReceiveProtocolBNFTP(SocketAsyncEventArgs e) { - if (ReceiveBuffer.Length == 0) return; - BNFTPState.Receive(ReceiveBuffer); + lock (_receiveLock) + { + if (ReceiveBuffer.Length == 0) return; + BNFTPState.Receive(ReceiveBuffer); + } } protected void ReceiveProtocolChat(SocketAsyncEventArgs e) { // TODO: Move the protocol parsing part of this function somewhere else under Protocols/ChatGateway (similar to Protocols/Game, etc.) - string text; - try + lock (_receiveLock) { - text = Encoding.UTF8.GetString(ReceiveBuffer); - } - catch (DecoderFallbackException) - { - Logging.WriteLine(Logging.LogLevel.Warning, Logging.LogType.Client_Chat, RemoteEndPoint, "Failed to decode UTF-8 text"); - Disconnect("Failed to decode UTF-8 text"); - return; - } - - // Mix alternate platform's new lines into our easily parsable NewLine constant: - //text = text.Replace("\r\n", "\n").Replace("\r", "\n").Replace("\n", Common.NewLine); - - while (text.Length > 0) - { - if (!text.Contains(Common.NewLine)) break; // Need more data from client + string text; + try + { + text = Encoding.UTF8.GetString(ReceiveBuffer); + } + catch (DecoderFallbackException) + { + Logging.WriteLine(Logging.LogLevel.Warning, Logging.LogType.Client_Chat, RemoteEndPoint, "Failed to decode UTF-8 text"); + Disconnect("Failed to decode UTF-8 text"); + return; + } - var pos = text.IndexOf(Common.NewLine); - ReceiveBuffer = ReceiveBuffer[(pos + Common.NewLine.Length)..]; - var line = text.Substring(0, pos); - text = text[(line.Length + Common.NewLine.Length)..]; + // Mix alternate platform's new lines into our easily parsable NewLine constant: + //text = text.Replace("\r\n", "\n").Replace("\r", "\n").Replace("\n", Common.NewLine); - if (GameState.ActiveAccount == null && string.IsNullOrEmpty(GameState.Username) && !string.IsNullOrEmpty(line) && line[0] == 0x04) + while (text.Length > 0) { - Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, "Client sent login byte [0x04]"); - line = line[1..]; + if (!text.Contains(Common.NewLine)) break; // Need more data from client - Send(Encoding.UTF8.GetBytes("Username: ")); - GameState.Username = null; - } + var pos = text.IndexOf(Common.NewLine); + ReceiveBuffer = ReceiveBuffer[(pos + Common.NewLine.Length)..]; + var line = text.Substring(0, pos); + text = text[(line.Length + Common.NewLine.Length)..]; - if (GameState.ActiveAccount == null && string.IsNullOrEmpty(GameState.Username) && !string.IsNullOrEmpty(line)) - { - GameState.Username = line; - Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Client set username to [{GameState.Username}]"); + if (GameState.ActiveAccount == null && string.IsNullOrEmpty(GameState.Username) && !string.IsNullOrEmpty(line) && line[0] == 0x04) + { + Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, "Client sent login byte [0x04]"); + line = line[1..]; - Send(Encoding.UTF8.GetBytes("Password: ")); - continue; - } + Send(Encoding.UTF8.GetBytes("Username: ")); + GameState.Username = null; + } - if (GameState.ActiveAccount == null) - { - var autoAccountCreate = Settings.GetBoolean(new string[] { "battlenet", "emulation", "chat_gateway", "auto_account_create" }, false); - var inPasswordHash = MBNCSUtil.XSha1.CalculateHash(Encoding.UTF8.GetBytes(line.ToLower())); - line = string.Empty; // prevent echoing password as a message if successfully authenticated + if (GameState.ActiveAccount == null && string.IsNullOrEmpty(GameState.Username) && !string.IsNullOrEmpty(line)) + { + GameState.Username = line; + Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Client set username to [{GameState.Username}]"); + + Send(Encoding.UTF8.GetBytes("Password: ")); + continue; + } - if (!Common.AccountsDb.TryGetValue(GameState.Username, out Account account) || account == null) + if (GameState.ActiveAccount == null) { - if (!autoAccountCreate) + var autoAccountCreate = Settings.GetBoolean(new string[] { "battlenet", "emulation", "chat_gateway", "auto_account_create" }, false); + var inPasswordHash = MBNCSUtil.XSha1.CalculateHash(Encoding.UTF8.GetBytes(line.ToLower())); + line = string.Empty; // prevent echoing password as a message if successfully authenticated + + if (!Common.AccountsDb.TryGetValue(GameState.Username, out Account account) || account == null) { - Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, "Client sent non-existent username"); - Send(Encoding.UTF8.GetBytes($"Incorrect username/password.{Common.NewLine}")); - continue; + if (!autoAccountCreate) + { + Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, "Client sent non-existent username"); + Send(Encoding.UTF8.GetBytes($"Incorrect username/password.{Common.NewLine}")); + continue; + } } - } - if (autoAccountCreate && account == null) - { - Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Creating account [{GameState.Username}] automatically for chat gateway client"); - Account.CreateStatus status = Account.TryCreate(GameState.Username, inPasswordHash, out account); - if (account == null || status != Account.CreateStatus.Success) + if (autoAccountCreate && account == null) { - var message = "Incorrect username/password"; - switch(status) + Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Creating account [{GameState.Username}] automatically for chat gateway client"); + Account.CreateStatus status = Account.TryCreate(GameState.Username, inPasswordHash, out account); + if (account == null || status != Account.CreateStatus.Success) { - case Account.CreateStatus.AccountExists: - message = "Account already exists"; break; - case Account.CreateStatus.LastCreateInProgress: - message = "Last create in progress"; break; - case Account.CreateStatus.UsernameAdjacentPunctuation: - message = "Username has adjacent punctuation"; break; - case Account.CreateStatus.UsernameBannedWord: - message = "Username contains a banned word"; break; - case Account.CreateStatus.UsernameInvalidChars: - message = "Username contains an invalid character"; break; - case Account.CreateStatus.UsernameShortAlphanumeric: - message = "Username contains too few alphanumeric characters"; break; - case Account.CreateStatus.UsernameTooManyPunctuation: - message = "Username contains too many punctuation characters"; break; - case Account.CreateStatus.UsernameTooShort: - message = "Username is too short"; break; + var message = "Incorrect username/password"; + switch(status) + { + case Account.CreateStatus.AccountExists: + message = "Account already exists"; break; + case Account.CreateStatus.LastCreateInProgress: + message = "Last create in progress"; break; + case Account.CreateStatus.UsernameAdjacentPunctuation: + message = "Username has adjacent punctuation"; break; + case Account.CreateStatus.UsernameBannedWord: + message = "Username contains a banned word"; break; + case Account.CreateStatus.UsernameInvalidChars: + message = "Username contains an invalid character"; break; + case Account.CreateStatus.UsernameShortAlphanumeric: + message = "Username contains too few alphanumeric characters"; break; + case Account.CreateStatus.UsernameTooManyPunctuation: + message = "Username contains too many punctuation characters"; break; + case Account.CreateStatus.UsernameTooShort: + message = "Username is too short"; break; + } + + Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"[{message}]"); + Send(Encoding.UTF8.GetBytes($"{message}.{Common.NewLine}")); + continue; } + else + { + Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Created account [{account.Get(Account.UsernameKey, GameState.Username)}] automatically for chat gateway client"); + } + } - Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"[{message}]"); - Send(Encoding.UTF8.GetBytes($"{message}.{Common.NewLine}")); + var dbPasswordHash = (byte[])account.Get(Account.PasswordKey, new byte[20]); + if (!inPasswordHash.SequenceEqual(dbPasswordHash)) + { + Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Incorrect password for account [{account.Get(Account.UsernameKey, GameState.Username)}]"); + Send(Encoding.UTF8.GetBytes($"Incorrect username/password.{Common.NewLine}")); continue; } - else + + var flags = (Account.Flags)account.Get(Account.FlagsKey, Account.Flags.None); + if ((flags & Account.Flags.Closed) != 0) { - Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Created account [{account.Get(Account.UsernameKey, GameState.Username)}] automatically for chat gateway client"); + Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Account [{account.Get(Account.UsernameKey, GameState.Username)}] is closed"); + Send(Encoding.UTF8.GetBytes($"Account closed.{Common.NewLine}")); + continue; } - } - var dbPasswordHash = (byte[])account.Get(Account.PasswordKey, new byte[20]); - if (!inPasswordHash.SequenceEqual(dbPasswordHash)) - { - Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Incorrect password for account [{account.Get(Account.UsernameKey, GameState.Username)}]"); - Send(Encoding.UTF8.GetBytes($"Incorrect username/password.{Common.NewLine}")); - continue; - } + Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Successfully authenticated into account [{account.Get(Account.UsernameKey, GameState.Username)}]"); - var flags = (Account.Flags)account.Get(Account.FlagsKey, Account.Flags.None); - if ((flags & Account.Flags.Closed) != 0) - { - Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Account [{account.Get(Account.UsernameKey, GameState.Username)}] is closed"); - Send(Encoding.UTF8.GetBytes($"Account closed.{Common.NewLine}")); - continue; - } + lock (GameState) + { + GameState.ActiveAccount = account; + GameState.LastLogon = (DateTime)account.Get(Account.LastLogonKey, DateTime.Now); - Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Chat, $"Successfully authenticated into account [{account.Get(Account.UsernameKey, GameState.Username)}]"); + account.Set(Account.IPAddressKey, RemoteEndPoint.ToString().Split(":")[0]); + account.Set(Account.LastLogonKey, DateTime.Now); + account.Set(Account.PortKey, RemoteEndPoint.ToString().Split(":")[1]); - lock (GameState) - { - GameState.ActiveAccount = account; - GameState.LastLogon = (DateTime)account.Get(Account.LastLogonKey, DateTime.Now); + var serial = 1; + var onlineName = GameState.Username; + while (!Common.ActiveAccounts.TryAdd(onlineName, account)) onlineName = $"{GameState.Username}#{++serial}"; + GameState.OnlineName = onlineName; + GameState.Username = (string)account.Get(Account.UsernameKey, GameState.Username); - account.Set(Account.IPAddressKey, RemoteEndPoint.ToString().Split(":")[0]); - account.Set(Account.LastLogonKey, DateTime.Now); - account.Set(Account.PortKey, RemoteEndPoint.ToString().Split(":")[1]); + GameState.LastPing = DateTime.Now; + GameState.LastPong = GameState.LastPing; + GameState.Ping = 0; + GameState.UDPSupported = false; // SID_ENTERCHAT will set NoUDP flag later. - var serial = 1; - var onlineName = GameState.Username; - while (!Common.ActiveAccounts.TryAdd(onlineName, account)) onlineName = $"{GameState.Username}#{++serial}"; - GameState.OnlineName = onlineName; - GameState.Username = (string)account.Get(Account.UsernameKey, GameState.Username); + GameState.Statstring = new byte[1]; + } - GameState.LastPing = DateTime.Now; - GameState.LastPong = GameState.LastPing; - GameState.Ping = 0; - GameState.UDPSupported = false; // SID_ENTERCHAT will set NoUDP flag later. + if (!Battlenet.Common.ActiveGameStates.TryAdd(GameState.OnlineName, GameState)) + { + Logging.WriteLine(Logging.LogLevel.Error, Logging.LogType.Client_Chat, RemoteEndPoint, $"Failed to add game state to active game state cache"); + account.Set(Account.FailedLogonsKey, ((UInt32)account.Get(Account.FailedLogonsKey, (UInt32)0)) + 1); + Battlenet.Common.ActiveAccounts.TryRemove(GameState.OnlineName, out _); + Send(Encoding.UTF8.GetBytes($"Incorrect username/password.{Common.NewLine}")); + continue; + } - GameState.Statstring = new byte[1]; - } + using var m1 = new MemoryStream(128); + using var w1 = new BinaryWriter(m1); + { + w1.Write(GameState.OnlineName); + w1.Write(GameState.Statstring); - if (!Battlenet.Common.ActiveGameStates.TryAdd(GameState.OnlineName, GameState)) - { - Logging.WriteLine(Logging.LogLevel.Error, Logging.LogType.Client_Chat, RemoteEndPoint, $"Failed to add game state to active game state cache"); - account.Set(Account.FailedLogonsKey, ((UInt32)account.Get(Account.FailedLogonsKey, (UInt32)0)) + 1); - Battlenet.Common.ActiveAccounts.TryRemove(GameState.OnlineName, out _); - Send(Encoding.UTF8.GetBytes($"Incorrect username/password.{Common.NewLine}")); - continue; - } + new SID_ENTERCHAT(m1.ToArray()).Invoke(new MessageContext(this, Protocols.MessageDirection.ClientToServer, + new Dictionary{{ "username", GameState.Username }, { "statstring", GameState.Statstring }}) + ); + } - using var m1 = new MemoryStream(128); - using var w1 = new BinaryWriter(m1); - { - w1.Write(GameState.OnlineName); - w1.Write(GameState.Statstring); + using var m2 = new MemoryStream(128); + using var w2 = new BinaryWriter(m2); + { + w2.Write((UInt32)SID_JOINCHANNEL.Flags.First); + w2.Write(Product.ProductChannelName(GameState.Product)); - new SID_ENTERCHAT(m1.ToArray()).Invoke(new MessageContext(this, Protocols.MessageDirection.ClientToServer, - new Dictionary{{ "username", GameState.Username }, { "statstring", GameState.Statstring }}) - ); + new SID_JOINCHANNEL(m2.ToArray()).Invoke(new MessageContext(this, Protocols.MessageDirection.ClientToServer)); + } } - using var m2 = new MemoryStream(128); - using var w2 = new BinaryWriter(m2); + if (string.IsNullOrEmpty(line)) continue; + + using var m3 = new MemoryStream(1 + Encoding.UTF8.GetByteCount(line)); + using var w3 = new BinaryWriter(m3); { - w2.Write((UInt32)SID_JOINCHANNEL.Flags.First); - w2.Write(Product.ProductChannelName(GameState.Product)); + w3.Write(line); - new SID_JOINCHANNEL(m2.ToArray()).Invoke(new MessageContext(this, Protocols.MessageDirection.ClientToServer)); + new SID_CHATCOMMAND(m3.ToArray()).Invoke(new MessageContext(this, Protocols.MessageDirection.ClientToServer)); } } - - if (string.IsNullOrEmpty(line)) continue; - - using var m3 = new MemoryStream(1 + Encoding.UTF8.GetByteCount(line)); - using var w3 = new BinaryWriter(m3); - { - w3.Write(line); - - new SID_CHATCOMMAND(m3.ToArray()).Invoke(new MessageContext(this, Protocols.MessageDirection.ClientToServer)); - } } } protected void ReceiveProtocolGame(SocketAsyncEventArgs e) { - byte[] newBuffer; - - while (ReceiveBuffer.Length > 0) + lock (_receiveLock) { - if (ReceiveBuffer.Length < 4) return; // Partial message header + byte[] newBuffer; - UInt16 messageLen = (UInt16)((ReceiveBuffer[3] << 8) + ReceiveBuffer[2]); + while (ReceiveBuffer.Length > 0) + { + if (ReceiveBuffer.Length < 4) return; // Partial message header - if (ReceiveBuffer.Length < messageLen) return; // Partial message + UInt16 messageLen = (UInt16)((ReceiveBuffer[3] << 8) + ReceiveBuffer[2]); - //byte messagePad = ReceiveBuffer[0]; // This is checked in the Message.FromByteArray() call. - byte messageId = ReceiveBuffer[1]; - byte[] messageBuffer = new byte[messageLen - 4]; - Buffer.BlockCopy(ReceiveBuffer, 4, messageBuffer, 0, messageLen - 4); + if (ReceiveBuffer.Length < messageLen) return; // Partial message - // Pop message off the receive buffer - newBuffer = new byte[ReceiveBuffer.Length - messageLen]; - Buffer.BlockCopy(ReceiveBuffer, messageLen, newBuffer, 0, ReceiveBuffer.Length - messageLen); - ReceiveBuffer = newBuffer; + //byte messagePad = ReceiveBuffer[0]; // This is checked in the Message.FromByteArray() call. + byte messageId = ReceiveBuffer[1]; + byte[] messageBuffer = new byte[messageLen - 4]; + Buffer.BlockCopy(ReceiveBuffer, 4, messageBuffer, 0, messageLen - 4); - // Push message onto stack - Message message = Message.FromByteArray(messageId, messageBuffer); + // Pop message off the receive buffer + newBuffer = new byte[ReceiveBuffer.Length - messageLen]; + Buffer.BlockCopy(ReceiveBuffer, messageLen, newBuffer, 0, ReceiveBuffer.Length - messageLen); + ReceiveBuffer = newBuffer; - if (message is Message) - { - BattlenetGameFrame.Messages.Enqueue(message); - continue; - } - else - { - throw new GameProtocolException(this, $"Received unknown SID_0x{messageId:X2} ({messageLen} bytes)"); + // Push message onto stack + Message message = Message.FromByteArray(messageId, messageBuffer); + + if (message is Message) + { + BattlenetGameFrame.Messages.Enqueue(message); + continue; + } + else + { + throw new GameProtocolException(this, $"Received unknown SID_0x{messageId:X2} ({messageLen} bytes)"); + } } - } - Invoke(e); + Invoke(e); + } } public void Send(byte[] buffer) From 35a349b53592e4cd9cf8258dcb55f8666fb8a03e Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:18:09 -0500 Subject: [PATCH 06/10] Fix ActiveGameAds race conditions with consistent locking ActiveGameAds was a List accessed from multiple threads with inconsistent locking. Add a dedicated ActiveGameAdsLock object and use it at all access sites. SID_STOPADV previously had no lock at all. Co-Authored-By: Claude Opus 4.6 --- src/Atlasd/Battlenet/Common.cs | 1 + src/Atlasd/Battlenet/Protocols/Game/Messages/SID_ENTERCHAT.cs | 2 +- .../Battlenet/Protocols/Game/Messages/SID_GETADVLISTEX.cs | 2 +- src/Atlasd/Battlenet/Protocols/Game/Messages/SID_NOTIFYJOIN.cs | 2 +- src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX.cs | 2 +- src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX2.cs | 2 +- src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX3.cs | 2 +- src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STOPADV.cs | 2 +- 8 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Atlasd/Battlenet/Common.cs b/src/Atlasd/Battlenet/Common.cs index f8318235..548f1837 100644 --- a/src/Atlasd/Battlenet/Common.cs +++ b/src/Atlasd/Battlenet/Common.cs @@ -43,6 +43,7 @@ public ShutdownEvent(string adminMessage, bool cancelled, DateTime eventDate, Ti public static ConcurrentDictionary ActiveClans; public static ConcurrentDictionary ActiveClientStates; public static List ActiveGameAds; + public static readonly object ActiveGameAdsLock = new object(); public static ConcurrentDictionary ActiveGameStates; public static IPAddress DefaultAddress { get; private set; } public static int DefaultPort { get; private set; } diff --git a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_ENTERCHAT.cs b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_ENTERCHAT.cs index f83dc618..ca9b77e1 100644 --- a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_ENTERCHAT.cs +++ b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_ENTERCHAT.cs @@ -102,7 +102,7 @@ public override bool Invoke(MessageContext context) { var gameAd = gameState.GameAd; if (gameAd.RemoveClient(gameState)) gameState.GameAd = null; - if (gameAd.Clients.Count == 0) lock (Battlenet.Common.ActiveGameAds) Battlenet.Common.ActiveGameAds.Remove(gameAd); + if (gameAd.Clients.Count == 0) lock (Battlenet.Common.ActiveGameAdsLock) Battlenet.Common.ActiveGameAds.Remove(gameAd); } if (gameState.ActiveChannel == null) diff --git a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_GETADVLISTEX.cs b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_GETADVLISTEX.cs index 6b915376..d573467f 100644 --- a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_GETADVLISTEX.cs +++ b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_GETADVLISTEX.cs @@ -58,7 +58,7 @@ public override bool Invoke(MessageContext context) var gameAds = new List(); - lock (Battlenet.Common.ActiveGameAds) + lock (Battlenet.Common.ActiveGameAdsLock) { IList toDelete = new List(); foreach (var gameAd in Battlenet.Common.ActiveGameAds) diff --git a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_NOTIFYJOIN.cs b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_NOTIFYJOIN.cs index b9efe3fc..3d257a6d 100644 --- a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_NOTIFYJOIN.cs +++ b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_NOTIFYJOIN.cs @@ -52,7 +52,7 @@ public override bool Invoke(MessageContext context) if (gameState.ActiveChannel != null) gameState.ActiveChannel.RemoveUser(gameState); - lock (Battlenet.Common.ActiveGameAds) + lock (Battlenet.Common.ActiveGameAdsLock) { foreach (var gameAd in Battlenet.Common.ActiveGameAds) { diff --git a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX.cs b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX.cs index 46c7816c..3971d1e0 100644 --- a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX.cs +++ b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX.cs @@ -69,7 +69,7 @@ public override bool Invoke(MessageContext context) Statuses status = Statuses.Failed; GameAd gameAd = null; - lock (Battlenet.Common.ActiveGameAds) + lock (Battlenet.Common.ActiveGameAdsLock) { foreach (GameAd _ad in Battlenet.Common.ActiveGameAds) { diff --git a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX2.cs b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX2.cs index 5c07f353..99f56651 100644 --- a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX2.cs +++ b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX2.cs @@ -69,7 +69,7 @@ public override bool Invoke(MessageContext context) Statuses status = Statuses.Failed; GameAd gameAd = null; - lock (Battlenet.Common.ActiveGameAds) + lock (Battlenet.Common.ActiveGameAdsLock) { foreach (GameAd _ad in Battlenet.Common.ActiveGameAds) { diff --git a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX3.cs b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX3.cs index 17806a3d..e51fe5e8 100644 --- a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX3.cs +++ b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX3.cs @@ -73,7 +73,7 @@ public override bool Invoke(MessageContext context) Statuses status = Statuses.Error; GameAd gameAd = null; - lock (Battlenet.Common.ActiveGameAds) + lock (Battlenet.Common.ActiveGameAdsLock) { foreach (GameAd _ad in Battlenet.Common.ActiveGameAds) { diff --git a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STOPADV.cs b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STOPADV.cs index 2f8a65f8..52039237 100644 --- a/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STOPADV.cs +++ b/src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STOPADV.cs @@ -42,7 +42,7 @@ public override bool Invoke(MessageContext context) if (!gameAdOwner) Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Game, context.Client.RemoteEndPoint, $"{MessageName(Id)} was received but they are not the owner of the game advertisement"); else - Battlenet.Common.ActiveGameAds.Remove(gs.GameAd); + lock (Battlenet.Common.ActiveGameAdsLock) Battlenet.Common.ActiveGameAds.Remove(gs.GameAd); return true; } From dbef8863cbd68cd2ebef3fa959e46a543b7d3c20 Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:19:07 -0500 Subject: [PATCH 07/10] Fix Channel.Designate race condition on DesignatedHeirs Designate() wrote to the dictionary without a lock while RemoveUser() locked it, creating a data race. Add matching lock. Co-Authored-By: Claude Opus 4.6 --- src/Atlasd/Battlenet/Channel.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Atlasd/Battlenet/Channel.cs b/src/Atlasd/Battlenet/Channel.cs index 5c38ea9a..afcec30b 100644 --- a/src/Atlasd/Battlenet/Channel.cs +++ b/src/Atlasd/Battlenet/Channel.cs @@ -285,7 +285,10 @@ public void Close() public void Designate(GameState designator, GameState heir) { - DesignatedHeirs[designator] = heir; + lock (DesignatedHeirs) + { + DesignatedHeirs[designator] = heir; + } } public bool DisbandInto(Channel destination) From b48d77b2eb57483b1831d28c8dbe36113d4f6f95 Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:20:27 -0500 Subject: [PATCH 08/10] Fix null race conditions in GameState.Close with local captures ActiveChannel, ActiveClan, ActiveAccount, and GameAd could be set to null by another thread between the null check and the method call. Use local variable capture to prevent NullReferenceException. Co-Authored-By: Claude Opus 4.6 --- .../Battlenet/Protocols/Game/GameState.cs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/Atlasd/Battlenet/Protocols/Game/GameState.cs b/src/Atlasd/Battlenet/Protocols/Game/GameState.cs index 93e74db2..38edbc18 100644 --- a/src/Atlasd/Battlenet/Protocols/Game/GameState.cs +++ b/src/Atlasd/Battlenet/Protocols/Game/GameState.cs @@ -19,7 +19,7 @@ public enum LogonTypes : UInt32 NLS = 2, }; - private bool IsDisposing = false; + private volatile bool IsDisposing = false; public ClientState Client { get; protected set; } @@ -126,26 +126,29 @@ public static bool CanStatstringUpdate(Product.ProductCode code) public void Close() { // Remove this GameState from ActiveChannel - if (ActiveChannel != null) + var channel = ActiveChannel; + if (channel != null) { - ActiveChannel.RemoveUser(this); // will change this.ActiveChannel to null. + channel.RemoveUser(this); // will change this.ActiveChannel to null. } // Notify clan members - if (ActiveClan != null) + var clan = ActiveClan; + if (clan != null) { - ActiveClan.WriteStatusChange(this, false); // offline + clan.WriteStatusChange(this, false); // offline } // Update keys of ActiveAccount - if (ActiveAccount != null) + var account = ActiveAccount; + if (account != null) { - ActiveAccount.Set(Account.LastLogoffKey, DateTime.Now); + account.Set(Account.LastLogoffKey, DateTime.Now); - var timeLogged = (UInt32)ActiveAccount.Get(Account.TimeLoggedKey); + var timeLogged = (UInt32)account.Get(Account.TimeLoggedKey); var diff = DateTime.Now - ConnectedTimestamp; timeLogged += (UInt32)Math.Round(diff.TotalSeconds); - ActiveAccount.Set(Account.TimeLoggedKey, timeLogged); + account.Set(Account.TimeLoggedKey, timeLogged); } // Remove this OnlineName from ActiveAccounts and ActiveGameStates @@ -156,7 +159,8 @@ public void Close() } // Remove this GameAd - if (GameAd != null && GameAd.RemoveClient(this)) GameAd = null; + var gameAd = GameAd; + if (gameAd != null && gameAd.RemoveClient(this)) GameAd = null; } public void Dispose() /* part of IDisposable */ From f8120d363fb4ae855be5c9aa9d19aeabe827d689 Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:22:42 -0500 Subject: [PATCH 09/10] Improve logging for malformed packet diagnosis The existing exception handling in SocketIOCompleted already catches all exceptions from message handlers and disconnects the client, so the server does not crash on malformed packets. However, the log output was unhelpful (e.g. "ArgumentOutOfRangeException error encountered!" with no packet context). Add try/catch in Invoke() that logs the message ID, size, and hex dump before disconnecting. Fix BinaryReader.ReadByteString() to throw a descriptive GameProtocolViolationException on missing null terminators instead of an opaque ArgumentOutOfRangeException. Fix GetNextNull() to restore stream position on failure. Co-Authored-By: Claude Opus 4.6 --- src/Atlasd/Battlenet/BinaryReader.cs | 16 ++++++++++++---- src/Atlasd/Battlenet/ClientState.cs | 14 +++++++++++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/Atlasd/Battlenet/BinaryReader.cs b/src/Atlasd/Battlenet/BinaryReader.cs index 93b8f4c8..c7baf624 100644 --- a/src/Atlasd/Battlenet/BinaryReader.cs +++ b/src/Atlasd/Battlenet/BinaryReader.cs @@ -1,4 +1,5 @@ -using System.IO; +using Atlasd.Battlenet.Exceptions; +using System.IO; using System.Text; namespace Atlasd.Battlenet @@ -17,7 +18,7 @@ public long GetNextNull() { long lastPosition = BaseStream.Position; - while (BaseStream.CanRead) + while (BaseStream.Position < BaseStream.Length) { if (ReadByte() == 0) { @@ -27,6 +28,7 @@ public long GetNextNull() } } + BaseStream.Position = lastPosition; return -1; } } @@ -35,7 +37,13 @@ public byte[] ReadByteString() { lock (_lock) { - var size = GetNextNull() - BaseStream.Position; + var nullPos = GetNextNull(); + if (nullPos < 0) + { + throw new GameProtocolViolationException(null, + $"Truncated string field at stream position {BaseStream.Position}: missing null terminator"); + } + var size = nullPos - BaseStream.Position; return ReadBytes((int)size)[..^1]; } } @@ -46,7 +54,7 @@ public override string ReadString() { string str = ""; char chr; - while ((int)(chr = ReadChar()) != 0) + while (BaseStream.Position < BaseStream.Length && (int)(chr = ReadChar()) != 0) str += chr; return str; } diff --git a/src/Atlasd/Battlenet/ClientState.cs b/src/Atlasd/Battlenet/ClientState.cs index f23b8ac1..adf116cd 100644 --- a/src/Atlasd/Battlenet/ClientState.cs +++ b/src/Atlasd/Battlenet/ClientState.cs @@ -139,8 +139,20 @@ private void Invoke(SocketAsyncEventArgs e) while (BattlenetGameFrame.Messages.TryDequeue(out var msg)) { - if (!msg.Invoke(context)) + try + { + if (!msg.Invoke(context)) + { + Disconnect(); + return; + } + } + catch (Exception ex) { + var hexDump = msg.Buffer != null ? BitConverter.ToString(msg.Buffer) : "(null)"; + Logging.WriteLine(Logging.LogLevel.Error, Logging.LogType.Client, RemoteEndPoint, + $"Exception processing {Message.MessageName(msg.Id)} (0x{msg.Id:X2}, {msg.Buffer?.Length ?? 0} bytes): " + + $"{ex.GetType().Name}: {ex.Message}; packet dump: {hexDump}"); Disconnect(); return; } From 419321a4374e07d80f2f15fbe45e5945ff2c34fe Mon Sep 17 00:00:00 2001 From: William LaFrance Date: Mon, 16 Mar 2026 20:31:05 -0500 Subject: [PATCH 10/10] Fix GameState.Close teardown with lock instead of local captures Local variable captures only prevented NullReferenceException but not the logical race (double cleanup if two threads call Close concurrently). Use lock(this) to make the entire teardown atomic. This is safe because Channel.RemoveUser already locks on the GameState (reentrant), and no other code path holds a different lock before calling Close. Co-Authored-By: Claude Opus 4.6 --- .../Battlenet/Protocols/Game/GameState.cs | 59 +++++++++---------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/src/Atlasd/Battlenet/Protocols/Game/GameState.cs b/src/Atlasd/Battlenet/Protocols/Game/GameState.cs index 38edbc18..76ee7840 100644 --- a/src/Atlasd/Battlenet/Protocols/Game/GameState.cs +++ b/src/Atlasd/Battlenet/Protocols/Game/GameState.cs @@ -125,42 +125,41 @@ public static bool CanStatstringUpdate(Product.ProductCode code) public void Close() { - // Remove this GameState from ActiveChannel - var channel = ActiveChannel; - if (channel != null) + lock (this) { - channel.RemoveUser(this); // will change this.ActiveChannel to null. - } + // Remove this GameState from ActiveChannel + if (ActiveChannel != null) + { + ActiveChannel.RemoveUser(this); // will change this.ActiveChannel to null. + } - // Notify clan members - var clan = ActiveClan; - if (clan != null) - { - clan.WriteStatusChange(this, false); // offline - } + // Notify clan members + if (ActiveClan != null) + { + ActiveClan.WriteStatusChange(this, false); // offline + } - // Update keys of ActiveAccount - var account = ActiveAccount; - if (account != null) - { - account.Set(Account.LastLogoffKey, DateTime.Now); + // Update keys of ActiveAccount + if (ActiveAccount != null) + { + ActiveAccount.Set(Account.LastLogoffKey, DateTime.Now); - var timeLogged = (UInt32)account.Get(Account.TimeLoggedKey); - var diff = DateTime.Now - ConnectedTimestamp; - timeLogged += (UInt32)Math.Round(diff.TotalSeconds); - account.Set(Account.TimeLoggedKey, timeLogged); - } + var timeLogged = (UInt32)ActiveAccount.Get(Account.TimeLoggedKey); + var diff = DateTime.Now - ConnectedTimestamp; + timeLogged += (UInt32)Math.Round(diff.TotalSeconds); + ActiveAccount.Set(Account.TimeLoggedKey, timeLogged); + } - // Remove this OnlineName from ActiveAccounts and ActiveGameStates - if (!string.IsNullOrEmpty(OnlineName)) - { - Battlenet.Common.ActiveAccounts.TryRemove(OnlineName, out _); - Battlenet.Common.ActiveGameStates.TryRemove(OnlineName, out _); - } + // Remove this OnlineName from ActiveAccounts and ActiveGameStates + if (!string.IsNullOrEmpty(OnlineName)) + { + Battlenet.Common.ActiveAccounts.TryRemove(OnlineName, out _); + Battlenet.Common.ActiveGameStates.TryRemove(OnlineName, out _); + } - // Remove this GameAd - var gameAd = GameAd; - if (gameAd != null && gameAd.RemoveClient(this)) GameAd = null; + // Remove this GameAd + if (GameAd != null && GameAd.RemoveClient(this)) GameAd = null; + } } public void Dispose() /* part of IDisposable */