From 69de57bcf2677238bbfb88e601bc38b89dc11dc0 Mon Sep 17 00:00:00 2001 From: Natalia Kondratyeva Date: Wed, 18 Aug 2021 10:01:59 +0200 Subject: [PATCH] Throw on invalid payload length in WebSockets (#57598) Port of https://github.com/dotnet/runtime/commit/9eb56805a13851c756d70a4cff4bd71fa89736f4 **Description:** Avoid integer overflow to prevent infinite loop in reading from WebSocket. (also complies better with WebSocket RFC) MSRC 65273 - Prevents DoS attack by sending frames with invalid payload length. **Risk:** Low **Impacted assemblies:** System.Net.WebSockets.dll --- .../src/Resources/Strings.resx | 5 ++- .../System/Net/WebSockets/ManagedWebSocket.cs | 8 +++++ .../tests/WebSocketCreateTest.cs | 31 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.WebSockets/src/Resources/Strings.resx b/src/libraries/System.Net.WebSockets/src/Resources/Strings.resx index 693f8d3863fd7..e96ac19e3f8ef 100644 --- a/src/libraries/System.Net.WebSockets/src/Resources/Strings.resx +++ b/src/libraries/System.Net.WebSockets/src/Resources/Strings.resx @@ -165,4 +165,7 @@ The compression options for a continuation cannot be different than the options used to send the first fragment of the message. - \ No newline at end of file + + The WebSocket received a frame with an invalid payload length. + + diff --git a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs index 05b171cf94ed7..ec62d92ac11a9 100644 --- a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs +++ b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs @@ -1116,6 +1116,14 @@ private async ValueTask CloseWithReceiveErrorAndThrowAsync( return SR.net_Websockets_ReservedBitsSet; } + if (header.PayloadLength < 0) + { + // as per RFC, if payload length is a 64-bit integer, the most significant bit MUST be 0 + // frame-payload-length-63 = %x0000000000000000-7FFFFFFFFFFFFFFF; 64 bits in length + resultHeader = default; + return SR.net_Websockets_InvalidPayloadLength; + } + if (header.Compressed && _inflater is null) { resultHeader = default; diff --git a/src/libraries/System.Net.WebSockets/tests/WebSocketCreateTest.cs b/src/libraries/System.Net.WebSockets/tests/WebSocketCreateTest.cs index f940df0add3ae..f9fd4e3c564e5 100644 --- a/src/libraries/System.Net.WebSockets/tests/WebSocketCreateTest.cs +++ b/src/libraries/System.Net.WebSockets/tests/WebSocketCreateTest.cs @@ -149,6 +149,37 @@ public async Task ReceiveAsync_InvalidFrameHeader_AbortsAndThrowsException(byte } } + [Theory] + [InlineData(new byte[] { 0x7F, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }, false)] // max allowed value + [InlineData(new byte[] { 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }, true)] + public async Task ReceiveAsync_InvalidPayloadLength_AbortsAndThrowsException(byte[] lenBytes, bool shouldFail) + { + var frame = new byte[11]; + frame[0] = 0b1_000_0010; // FIN, RSV, OPCODE + frame[1] = 0b0_1111111; // MASK, PAYLOAD_LEN + Assert.Equal(8, lenBytes.Length); + Array.Copy(lenBytes, 0, frame, 2, lenBytes.Length); // EXTENDED_PAYLOAD_LEN + frame[10] = (byte)'a'; + + using var stream = new MemoryStream(frame, writable: true); + using WebSocket websocket = CreateFromStream(stream, false, null, Timeout.InfiniteTimeSpan); + + var buffer = new byte[1]; + Task t = websocket.ReceiveAsync(new ArraySegment(buffer), CancellationToken.None); + if (shouldFail) + { + var exc = await Assert.ThrowsAsync(() => t); + Assert.Equal(WebSocketState.Aborted, websocket.State); + } + else + { + WebSocketReceiveResult result = await t; + Assert.False(result.EndOfMessage); + Assert.Equal(1, result.Count); + Assert.Equal('a', (char)buffer[0]); + } + } + [Fact] [SkipOnPlatform(TestPlatforms.Browser, "System.Net.Sockets is not supported on this platform.")] [ActiveIssue("https://github.com/dotnet/runtime/issues/34690", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]