crypto: fix TLSWrap use-after-free on pending write

EncOut() passes pointers from the enc_out_ BIO internal buffer to the
underlying stream's uv_write().  write_size_ is non-zero while that
write is in flight.  Calling ssl_.reset() frees the SSL context and
its BIOs, turning those pointers into dangling references.  When libuv
completes the write it accesses freed memory (SIGSEGV).

Use ssl_.release() instead of ssl_.reset() when write_size_ != 0 so
the BIO data stays alive for the in-flight uv_write.  This is a
bounded leak (one SSL context per socket destroyed with in-flight
writes) that prevents a segfault.

Also move RemoveStreamListener() before SSL cleanup so the underlying
stream cannot call back into the TLSWrap after its SSL state is gone.

Refs: #62393
Made-with: Cursor

Author: alexkozy

src/crypto/crypto_tls.cc

@@ -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();
 }
 

test/parallel/test-tls-destroy-ssl-with-pending-write.js

@@ -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) {
+          socket._handle.destroySSL();
+        }
+
+        setTimeout(() => {
+          socket.destroy();
+          if (--remaining === 0) {
+            server.close();
+          }
+        }, 50);
+      }),
+    );
+    socket.on('error', () => {});
+  }
+}));