Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw on invalid payload length in WebSockets #57598

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

CarnaViire
Copy link
Member

@CarnaViire CarnaViire commented Aug 17, 2021

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

@ghost
Copy link

ghost commented Aug 17, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: CarnaViire
Assignees: -
Labels:

area-System.Net

Milestone: -

@stephentoub
Copy link
Member

(If this doesn't make the RC1 snap, it'll need to be ported over.)

@karelz
Copy link
Member

karelz commented Aug 18, 2021

Failures are unrelated:

Process terminated. Assertion failed.
Exception occurred during handling Stream SHUTDOWN_COMPLETE event: System.NullReferenceException: Object reference not set to an instance of an object.
   at System.Net.Quic.Implementations.MsQuic.MsQuicStream.HandleEventConnectionClose(State state) in /_/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs:line 1368
   at System.Net.Quic.Implementations.MsQuic.MsQuicStream.HandleEventShutdownComplete(State state, StreamEvent& evt) in /_/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs:line 1024
   at System.Net.Quic.Implementations.MsQuic.MsQuicStream.HandleEvent(State state, StreamEvent& evt) in /_/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs:line 860
   at System.Net.Quic.Implementations.MsQuic.MsQuicStream.HandleEvent(State state, StreamEvent& evt) in /_/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs:line 860
   at System.Net.Quic.Implementations.MsQuic.MsQuicStream.NativeCallbackHandler(IntPtr stream, IntPtr context, StreamEvent& streamEvent) in /_/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs:line 824

@karelz karelz added this to the 7.0.0 milestone Aug 18, 2021
@karelz karelz merged commit 69de57b into dotnet:main Aug 18, 2021
@karelz
Copy link
Member

karelz commented Aug 18, 2021

@CarnaViire can you please create ports to branches release/6.0 and release/6.0-rc1? (we will send the 6.0-rc1 one to Tactics first)

@CarnaViire
Copy link
Member Author

/backport to release/6.0-rc1

@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc1: https://github.com/dotnet/runtime/actions/runs/1143099091

@CarnaViire
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1143161019

@CarnaViire
Copy link
Member Author

System.Net.Http.Funtional - Quic test failure (not yet filed)

@karelz it was actually filed, though it's not clear from issue's title #55815

@ghost ghost locked as resolved and limited conversation to collaborators Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants