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

Two fixes for TLSWrap::Destroy() when called with in-flight writes:

1. Destructor path (GC weak callback): do not call InvokeQueued().
   StreamReq::Done() mutates JS objects via v8::Object::Set() and may
   allocate on the V8 heap, violating V8's invariant that first-pass
   weak callbacks must not trigger heap mutation or nested GC.

2. BIO buffer use-after-free: EncOut() passes pointers from the
   enc_out_ BIO internal buffer to the underlying stream's uv_write().
   If ssl_.reset() frees the BIO while that write is still in flight,
   the pointers become dangling.  Use ssl_.release() instead when
   write_size_ != 0 so the BIO data stays alive.

Also move RemoveStreamListener() before SSL cleanup so the underlying
stream cannot call back into the destroyed TLSWrap.

Refs: #62393
Made-with: Cursor

Author: alexkozy

src/crypto/crypto_tls.cc

@@ -438,7 +438,10 @@ TLSWrap::TLSWrap(Environment* env,
 }
 
 TLSWrap::~TLSWrap() {
-  Destroy();
+  // Destructor may run from V8 weak callbacks during garbage collection.
+  // Do not invoke JS-visible write callbacks (StreamReq::Done mutates
+  // JS objects and may allocate on the V8 heap, violating GC invariants).
+  Destroy(false);
 }
 
 MaybeLocal<ArrayBufferView> TLSWrap::ocsp_response() const {
@@ -1307,25 +1310,37 @@ void TLSWrap::DestroySSL(const FunctionCallbackInfo<Value>& args) {
   Debug(wrap, "DestroySSL() finished");
 }
 
-void TLSWrap::Destroy() {
+void TLSWrap::Destroy(bool invoke_queued) {
   if (!ssl_)
     return;
 
-  // If there is a write happening, mark it as finished.
-  write_callback_scheduled_ = true;
+  if (invoke_queued) {
+    write_callback_scheduled_ = true;
+    InvokeQueued(UV_ECANCELED, "Canceled because of SSL destruction");
+  } else {
+    current_write_.reset();
+    current_empty_write_.reset();
+    write_callback_scheduled_ = false;
+  }
 
-  // And destroy
-  InvokeQueued(UV_ECANCELED, "Canceled because of SSL destruction");
+  // Detach from the underlying stream before releasing the SSL context.
+  if (underlying_stream() != nullptr)
+    underlying_stream()->RemoveStreamListener(this);
 
-  env()->external_memory_accounter()->Decrease(env()->isolate(), kExternalSize);
-  ssl_.reset();
+  // 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;
-
-  if (underlying_stream() != nullptr)
-    underlying_stream()->RemoveStreamListener(this);
-
   sc_.reset();
 }
 

src/crypto/crypto_tls.h

@@ -159,7 +159,7 @@ class TLSWrap : public AsyncWrap,
   void EncOut();  // Write encrypted data from enc_out_ to underlying stream.
   void ClearIn();  // SSL_write() clear data "in" to SSL.
   void ClearOut();  // SSL_read() clear text "out" from SSL.
-  void Destroy();
+  void Destroy(bool invoke_queued = true);
 
   // Call Done() on outstanding WriteWrap request.
   void InvokeQueued(int status, const char* error_str = nullptr);

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', () => {});
+  }
+}));