Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/crypto/crypto_tls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1317,15 +1317,24 @@ void TLSWrap::Destroy() {
// And destroy
InvokeQueued(UV_ECANCELED, "Canceled because of SSL destruction");

env()->external_memory_accounter()->Decrease(env()->isolate(), kExternalSize);
ssl_.reset();

enc_in_ = nullptr;
enc_out_ = nullptr;

// Detach from the underlying stream before releasing the SSL context.
if (underlying_stream() != nullptr)
underlying_stream()->RemoveStreamListener(this);

// EncOut() passes pointers into the enc_out_ BIO internal buffer to the
// underlying stream via uv_write(). write_size_ is non-zero while that
// write is in flight. Freeing the SSL context would turn those pointers
// into dangling references (use-after-free when libuv completes the
// write). Release ownership without freeing so the BIO data stays alive.
if (write_size_ != 0) {
ssl_.release();
} else {
ssl_.reset();
}

env()->external_memory_accounter()->Decrease(env()->isolate(), kExternalSize);
enc_in_ = nullptr;
enc_out_ = nullptr;
sc_.reset();
}

Expand Down
56 changes: 56 additions & 0 deletions test/parallel/test-tls-destroy-ssl-with-pending-write.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
'use strict';

// Regression test for TLSWrap use-after-free when destroySSL() is called
// while an underlying stream write is still in flight.
//
// EncOut() passes pointers into the enc_out_ BIO's internal buffer to the
// underlying stream via uv_write(). If the SSL context is freed before that
// write completes, libuv accesses freed memory (SIGSEGV).
//
// The GC weak callback path triggers the same Destroy() code when a
// TLSSocket with pending writes is collected without an explicit destroy().

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const fixtures = require('../common/fixtures');
const tls = require('node:tls');

const server = tls.createServer({
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem'),
}, (socket) => {
// Do not read — TCP backpressure keeps client writes pending in the
// TLSWrap's underlying stream so write_size_ stays non-zero.
socket.pause();
});

server.listen(0, common.mustCall(() => {
const { port } = server.address();
let remaining = 10;

for (let i = 0; i < 10; i++) {
const socket = tls.connect(
{ port, host: '127.0.0.1', rejectUnauthorized: false },
common.mustCall(() => {
// Queue a write large enough to stay pending (server never reads).
socket.write(Buffer.alloc(1024 * 1024));

// Simulate the GC weak callback path: free the SSL context while
// the underlying stream write is still in flight.
if (socket._handle && socket._handle.destroySSL) {

Check failure on line 42 in test/parallel/test-tls-destroy-ssl-with-pending-write.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Prefer optional chaining
socket._handle.destroySSL();
}

setTimeout(() => {
socket.destroy();
if (--remaining === 0) {
server.close();
}
}, 50);
}),
);
socket.on('error', () => {});
}
}));
Loading