From 6ff4f70f1a13d34b148c87a1dd64196db9f027dc Mon Sep 17 00:00:00 2001 From: gamer147 Date: Sun, 31 May 2026 21:46:02 -0400 Subject: [PATCH] fix(battle-node): SocketIoFrame disposal safety + escaping + empty-args encoding - Wrap all JsonDocument.Parse calls in using blocks and Clone() each retained JsonElement to eliminate UAF hazard after GC. - Use JsonSerializer.Serialize with UnsafeRelaxedJsonEscaping so event names with " or \ produce \" / \ rather than " / plain \; avoids malformed JSON on Encode(). - Guard the [ ] block in Encode() behind EventName-or-args check so Connect/Disconnect packets round-trip as bare "0"/"1" not "0[]". - Add three regression tests: Connect no-bracket, Event round-trip, special-char event name escaping. Co-Authored-By: Claude Sonnet 4.6 --- SVSim.BattleNode/Wire/SocketIoFrame.cs | 60 +++++++++++++------ .../BattleNode/Wire/SocketIoFrameTests.cs | 30 ++++++++++ 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/SVSim.BattleNode/Wire/SocketIoFrame.cs b/SVSim.BattleNode/Wire/SocketIoFrame.cs index 7001598..3a0b2d6 100644 --- a/SVSim.BattleNode/Wire/SocketIoFrame.cs +++ b/SVSim.BattleNode/Wire/SocketIoFrame.cs @@ -1,8 +1,17 @@ using System.Text; +using System.Text.Encodings.Web; using System.Text.Json; namespace SVSim.BattleNode.Wire; +file static class SocketIoJsonOptions +{ + internal static readonly JsonSerializerOptions EventNameOptions = new() + { + Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping + }; +} + /// /// Socket.IO v2 packet. Wire form: <type><N>-<ackId?>[json-args] where /// <N>- appears only on binary types (5/6). For binary events/acks, the JSON contains @@ -72,9 +81,16 @@ public sealed class SocketIoFrame } var argsJson = cursor < raw.Length ? raw.Substring(cursor) : string.Empty; - var allElements = string.IsNullOrEmpty(argsJson) - ? Array.Empty() - : JsonDocument.Parse(argsJson).RootElement.EnumerateArray().ToArray(); + JsonElement[] allElements; + if (string.IsNullOrEmpty(argsJson)) + { + allElements = Array.Empty(); + } + else + { + using var doc = JsonDocument.Parse(argsJson); + allElements = doc.RootElement.EnumerateArray().Select(el => el.Clone()).ToArray(); + } string? eventName = null; JsonElement[] rawArgs; @@ -119,7 +135,11 @@ public sealed class SocketIoFrame sb.Append("{\"_placeholder\":true,\"num\":").Append(i).Append('}'); } sb.Append(']'); - var args = JsonDocument.Parse(sb.ToString()).RootElement.EnumerateArray().ToArray(); + JsonElement[] args; + using (var doc = JsonDocument.Parse(sb.ToString())) + { + args = doc.RootElement.EnumerateArray().Select(el => el.Clone()).ToArray(); + } return new SocketIoFrame( SocketIoPacketType.BinaryEvent, @@ -133,7 +153,11 @@ public sealed class SocketIoFrame /// Build an ack response with a single int argument (the spec's pubSeq echo). public static SocketIoFrame AckResponse(int ackId, int arg) { - var args = JsonDocument.Parse($"[{arg}]").RootElement.EnumerateArray().ToArray(); + JsonElement[] args; + using (var doc = JsonDocument.Parse($"[{arg}]")) + { + args = doc.RootElement.EnumerateArray().Select(el => el.Clone()).ToArray(); + } return new SocketIoFrame( SocketIoPacketType.Ack, ackId, 0, null, args, Array.Empty()); } @@ -152,20 +176,22 @@ public sealed class SocketIoFrame } if (AckId.HasValue) sb.Append(AckId.Value); // Re-serialize args — for event/binary-event types, re-prepend the event name. - sb.Append('['); - var prependEventName = Type is SocketIoPacketType.Event or SocketIoPacketType.BinaryEvent - && EventName is not null; - if (prependEventName) + bool hasJsonPayload = EventName is not null || RawArgs.Length > 0; + if (hasJsonPayload) { - sb.Append('"').Append(EventName).Append('"'); - if (RawArgs.Length > 0) sb.Append(','); + sb.Append('['); + if (EventName is not null) + { + sb.Append(JsonSerializer.Serialize(EventName, SocketIoJsonOptions.EventNameOptions)); + if (RawArgs.Length > 0) sb.Append(','); + } + for (var i = 0; i < RawArgs.Length; i++) + { + if (i > 0) sb.Append(','); + sb.Append(RawArgs[i].GetRawText()); + } + sb.Append(']'); } - for (var i = 0; i < RawArgs.Length; i++) - { - if (i > 0) sb.Append(','); - sb.Append(RawArgs[i].GetRawText()); - } - sb.Append(']'); return (sb.ToString(), BinaryAttachments); } } diff --git a/SVSim.UnitTests/BattleNode/Wire/SocketIoFrameTests.cs b/SVSim.UnitTests/BattleNode/Wire/SocketIoFrameTests.cs index 841d731..9cf43c5 100644 --- a/SVSim.UnitTests/BattleNode/Wire/SocketIoFrameTests.cs +++ b/SVSim.UnitTests/BattleNode/Wire/SocketIoFrameTests.cs @@ -85,4 +85,34 @@ public class SocketIoFrameTests var header = SocketIoFrame.Parse("51-[\"msg\",{\"_placeholder\":true,\"num\":0}]"); Assert.Throws(() => header.WithAttachments(Array.Empty())); } + + [Test] + public void Encode_ConnectPacket_HasNoBracketedArgs() + { + // Regression for an earlier bug where Encode always emitted "[]". + var frame = SocketIoFrame.Parse("0"); + var (text, bins) = frame.Encode(); + Assert.That(text, Is.EqualTo("0")); + Assert.That(bins, Is.Empty); + } + + [Test] + public void ParseThenEncode_EventWithAck_RoundTripsByteForByte() + { + const string wire = "27[\"msg\",42]"; + var frame = SocketIoFrame.Parse(wire); + var (text, _) = frame.Encode(); + Assert.That(text, Is.EqualTo(wire)); + } + + [Test] + public void Encode_EventNameWithSpecialChars_IsJsonEscaped() + { + var frame = SocketIoFrame.BinaryEventWithAttachments( + eventName: "weird \"name\" with \\ backslash", + attachments: new[] { new byte[] { 0x00 } }); + var (text, _) = frame.Encode(); + // The event name must be JSON-escaped: each " becomes \", and the literal \ becomes \\. + Assert.That(text, Does.Contain("\"weird \\\"name\\\" with \\\\ backslash\"")); + } }