OpenSSL: fix use-after-free of encrypted send buffer in _SslSend#186
Open
Vizit0r wants to merge 1 commit into
Open
OpenSSL: fix use-after-free of encrypted send buffer in _SslSend#186Vizit0r wants to merge 1 commit into
Vizit0r wants to merge 1 commit into
Conversation
Problem ------- With the OpenSSL TLS client, the handshake completes but the first application-data SSL_read after sending fails with SSL_ERROR_SSL (empty error queue), and the connection drops. Reproducible and deterministic on macOS (Kqueue); latent on Linux (Epoll) and Windows (IOCP), where it usually works by luck. Root cause ---------- TCrossOpenSslConnection._SslSend transmitted the encrypted data via the pointer overload _Send(@LEncryptedData[0], Length(LEncryptedData), cb). The backends' Send does not copy the buffer -- it queues the raw pointer and defers the actual transmit to the write-ready event. But LEncryptedData is a local TBytes that the callback closure does not capture, so it was freed when _SslSend returned, before the deferred send ran. The transmit then read freed memory. When the allocator reuses that memory before transmit (deterministic on macOS), the client sends a corrupted TLS record; the peer can't decrypt it and replies with a fatal alert, surfacing as the failed SSL_read. The handshake is unaffected because handshake data is sent through the TBytes overload _Send(LHandshakeData), which keeps the buffer alive via closure capture. Fix --- Send the encrypted data through the TBytes overload as well, so the buffer is retained until the send callback fires. This is the canonical safe pattern -- TCrossConnectionBase.SendBytes (Net.CrossSocket.Base.pas ~ line 1821) -- where the author explicitly documents the trap in a comment: SendBuf passes a raw address and does not increment refcount, so the caller must keep a local reference. A sweep of all relevant call sites confirms every other caller (HTTP server, WebSocket -> SendBytes, MbedTls backend) already follows the contract; _SslSend in the OpenSSL backend was the only violation. Tested ------ HTTPS GET (372 KB body) -- PASS on Windows (IOCP), Linux (OpenSSL 3.0.13 / Epoll), macOS (OpenSSL 3.1.3 / Kqueue). macOS was failing deterministically before the change; no regression on the others.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
With the OpenSSL TLS client, the handshake completes but the first application-data SSL_read after sending fails with SSL_ERROR_SSL (empty error queue), and the connection drops. Reproducible and deterministic on macOS (Kqueue); latent on Linux (Epoll) and Windows (IOCP), where it usually works by luck.
Root cause
TCrossOpenSslConnection._SslSend transmitted the encrypted data via the pointer overload _Send(@LEncryptedData[0], Length(LEncryptedData), cb). The backends' Send does not copy the buffer -- it queues the raw pointer and defers the actual transmit to the write-ready event. But LEncryptedData is a local TBytes that the callback closure does not capture, so it was freed when _SslSend returned, before the deferred send ran. The transmit then read freed memory.
When the allocator reuses that memory before transmit (deterministic on macOS), the client sends a corrupted TLS record; the peer can't decrypt it and replies with a fatal alert, surfacing as the failed SSL_read. The handshake is unaffected because handshake data is sent through the TBytes overload _Send(LHandshakeData), which keeps the buffer alive via closure capture.
Fix
Send the encrypted data through the TBytes overload as well, so the buffer is retained until the send callback fires.
This is the canonical safe pattern -- TCrossConnectionBase.SendBytes (Net.CrossSocket.Base.pas ~ line 1821) -- where the author explicitly documents the trap in a comment: SendBuf passes a raw address and does not increment refcount, so the caller must keep a local reference. A sweep of all relevant call sites confirms every other caller (HTTP server, WebSocket -> SendBytes, MbedTls backend) already follows the contract; _SslSend in the OpenSSL backend was the only violation.
Tested
HTTPS GET (372 KB body) -- PASS on Windows (IOCP), Linux (OpenSSL 3.0.13 / Epoll), macOS (OpenSSL 3.1.3 / Kqueue). macOS was failing deterministically before the change; no regression on the others.
Resolves #176