Skip to content

Fix data race in SSLSocketStream causing TLS session corruption in WebSocket - #2550

Open
Hyukya wants to merge 2 commits into
yhirose:masterfrom
Hyukya:master
Open

Fix data race in SSLSocketStream causing TLS session corruption in WebSocket#2550
Hyukya wants to merge 2 commits into
yhirose:masterfrom
Hyukya:master

Conversation

@Hyukya

@Hyukya Hyukya commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Problem

SSLSocketStream can be shared across threads, allowing concurrent access to the same TLS session.
The WebSocket ping thread, the application's send(), and close()'s wait-for-response read all enter the same session.
The write path also reads (wait_writable() -> is_peer_closed() -> SSL_peek), so SSL_read and SSL_peek collide on the same record layer buffer, causing a buffer overflow.

This only affects wss://. ws:// doesn't use a TLS session, so it's unaffected.

Reproduction

Added test_websocket_thread_safety.cc. One thread loops read() while another calls send() / close().

  • CloseWhileAnotherThreadReads (macOS, ASan): heap-buffer-overflow via
    WebSocketClient::close() -> read_websocket_frame() -> SSLSocketStream::read() -> SSL_read() (during OpenSSL GCM cipher param handling).
  • SendWhileAnotherThreadReads (macOS/Linux): sent falls far short of the expected 2000 (macOS: 7, Linux: 19).
    On Linux, frames_read == 0 also failed.
    No crash — messages are silently dropped.

Being a race, the exact failure point varies per run; a single pass doesn't
prove safety.

Fix

Add one mutex per SSLSocketStream, held around every call that enters the
session: tls::pending, tls::read, tls::write, tls::is_peer_closed.
select_read/select_write stay outside the lock since they're socket
operations, not session operations. Locking is not applied at the WebSocket
layer, so an idle reader doesn't block a sender for the whole read timeout.

Out of scope

  • This mutex is a safety net around TLS calls, not a redesign of WebSocket
    I/O. std::mutex doesn't guarantee fairness, so send starvation under
    heavy read load is still possible. A single I/O owner with an outbound
    queue would be a better long-term structure.
  • The stack traces are from the OpenSSL backend; no claim is made about
    identical internal corruption on other backends.

Hyukya added 2 commits August 23, 2026 16:16
Protect TLS session operations in SSLSocketStream with a mutex.

This prevents races when WebSocket send and receive paths concurrently access
the same TLS session, including pending checks, peer-close detection, reads,
and writes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant