Skip to content

fix: cap websocket message buffering and remove stale websocket entries#42

Merged
CypherPotato merged 1 commit into
mainfrom
codex/propose-fix-for-websocket-dos-vulnerability
May 25, 2026
Merged

fix: cap websocket message buffering and remove stale websocket entries#42
CypherPotato merged 1 commit into
mainfrom
codex/propose-fix-for-websocket-dos-vulnerability

Conversation

@CypherPotato
Copy link
Copy Markdown
Member

Motivation

  • A previous WebSocket receive rewrite accumulated entire fragmented messages into memory with no size cap, allowing a remote client to trigger unbounded memory growth.
  • Disposal behavior no longer unregistered sockets, and collection operations disposed sockets without removing them, leaving stale entries that can amplify resource exhaustion.

Description

  • Enforced a maximum WebSocket message size in HttpWebSocket.ReceiveInternalAsync by reading request.baseServer.ServerConfiguration.MaximumContentLength and treating non-positive values as unlimited (Int32.MaxValue).
  • Added an early termination path that closes the socket with WebSocketCloseStatus.MessageTooBig and calls CloseAsync when an in-progress fragmented message would exceed the configured maximum before writing more data into the accumulator.
  • Fixed HttpWebSocketConnectionCollection.RegisterWebSocket to remove duplicate identified sockets from the internal _ws list before disposing them to avoid leaving stale references.
  • Fixed HttpWebSocketConnectionCollection.DropAll() to clear _ws and then dispose the previous entries so disposed sockets are not retained in the collection.

Testing

  • Ran dotnet test src/Sisk.Core.csproj -v minimal and the project tests/build succeeded.
  • Running dotnet test from the repository root fails when no solution/project file is specified, so the targeted project test was used and completed successfully.

Codex Task

@CypherPotato CypherPotato merged commit 2354f6f into main May 25, 2026
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant