From fe304dfe01ee4cf108a26730930990434b9aab92 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 23 Feb 2026 04:55:53 -0500 Subject: [PATCH] fix: review fixes for WsReadInfo and WsUpgrade - WsReadInfo: validate 64-bit frame payload length against maxPayload before casting to int (prevents overflow/memory exhaustion) - WsReadInfo: always send close response per RFC 6455 Section 5.5.1, including for empty close frames - WsUpgrade: restrict no-masking to leaf node connections only (browser clients must always mask frames) --- src/NATS.Server/WebSocket/WsReadInfo.cs | 11 ++++++++++- src/NATS.Server/WebSocket/WsUpgrade.cs | 5 +++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/NATS.Server/WebSocket/WsReadInfo.cs b/src/NATS.Server/WebSocket/WsReadInfo.cs index 2385f38..9dbbc29 100644 --- a/src/NATS.Server/WebSocket/WsReadInfo.cs +++ b/src/NATS.Server/WebSocket/WsReadInfo.cs @@ -176,7 +176,10 @@ public struct WsReadInfo { var (lenBuf, p2) = WsGet(stream, buf, pos, max, 8); pos = p2; - r.Remaining = (int)BinaryPrimitives.ReadUInt64BigEndian(lenBuf); + var len64 = BinaryPrimitives.ReadUInt64BigEndian(lenBuf); + if (len64 > (ulong)maxPayload) + throw new InvalidOperationException($"frame payload length {len64} exceeds max payload {maxPayload}"); + r.Remaining = (int)len64; break; } } @@ -264,11 +267,17 @@ public struct WsReadInfo if (payload.Length > WsConstants.CloseStatusSize) r.CloseBody = Encoding.UTF8.GetString(payload.AsSpan(WsConstants.CloseStatusSize)); } + // Per RFC 6455 Section 5.5.1, always send a close response if (r.CloseStatus != WsConstants.CloseStatusNoStatusReceived) { var closeMsg = WsFrameWriter.CreateCloseMessage(r.CloseStatus, r.CloseBody ?? ""); r.PendingControlFrames.Add(new ControlFrameAction(WsConstants.CloseMessage, closeMsg)); } + else + { + // Empty close frame — respond with empty close + r.PendingControlFrames.Add(new ControlFrameAction(WsConstants.CloseMessage, [])); + } break; case WsConstants.PingMessage: diff --git a/src/NATS.Server/WebSocket/WsUpgrade.cs b/src/NATS.Server/WebSocket/WsUpgrade.cs index ba91191..662065f 100644 --- a/src/NATS.Server/WebSocket/WsUpgrade.cs +++ b/src/NATS.Server/WebSocket/WsUpgrade.cs @@ -62,8 +62,9 @@ public static class WsUpgrade ext.Contains(WsConstants.PmcExtension, StringComparison.OrdinalIgnoreCase); } - // No-masking support - bool noMasking = headers.TryGetValue(WsConstants.NoMaskingHeader, out var nmVal) && + // No-masking support (leaf nodes only — browser clients must always mask) + bool noMasking = kind == WsClientKind.Leaf && + headers.TryGetValue(WsConstants.NoMaskingHeader, out var nmVal) && string.Equals(nmVal.Trim(), WsConstants.NoMaskingValue, StringComparison.OrdinalIgnoreCase); // Browser detection