Skip to content

Commit

Permalink
Throw on invalid payload length in WebSockets (#57598)
Browse files Browse the repository at this point in the history
Port of 9eb5680

**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
  • Loading branch information
CarnaViire authored Aug 18, 2021
1 parent ab0f54a commit 69de57b
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,7 @@
<data name="net_WebSockets_Argument_MessageFlagsHasDifferentCompressionOptions" xml:space="preserve">
<value>The compression options for a continuation cannot be different than the options used to send the first fragment of the message.</value>
</data>
</root>
<data name="net_Websockets_InvalidPayloadLength" xml:space="preserve">
<value>The WebSocket received a frame with an invalid payload length.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
31 changes: 31 additions & 0 deletions src/libraries/System.Net.WebSockets/tests/WebSocketCreateTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<WebSocketReceiveResult> t = websocket.ReceiveAsync(new ArraySegment<byte>(buffer), CancellationToken.None);
if (shouldFail)
{
var exc = await Assert.ThrowsAsync<WebSocketException>(() => 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)]
Expand Down

0 comments on commit 69de57b

Please sign in to comment.