From 31dc04e0a4cbcf6e8b97c90c1e9a27c3d72493e7 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Fri, 17 Mar 2017 15:31:14 +0100
Subject: [PATCH] tls: fix segfault on destroy after partial read

OnRead() calls into JS land which can result in the SSL context object
being destroyed on return.  Check that `ssl_ != nullptr` afterwards.

Fixes: https://github.com/nodejs/node/issues/11885
PR-URL: https://github.com/nodejs/node/pull/11898
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/tls_wrap.cc                          |  6 ++++
 test/parallel/test-tls-socket-destroy.js | 36 ++++++++++++++++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 test/parallel/test-tls-socket-destroy.js

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 39642cf0680709..d854fd8530a9ae 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -426,6 +426,12 @@ void TLSWrap::ClearOut() {
       memcpy(buf.base, current, avail);
       OnRead(avail, &buf);
 
+      // Caveat emptor: OnRead() calls into JS land which can result in
+      // the SSL context object being destroyed.  We have to carefully
+      // check that ssl_ != nullptr afterwards.
+      if (ssl_ == nullptr)
+        return;
+
       read -= avail;
       current += avail;
     }
diff --git a/test/parallel/test-tls-socket-destroy.js b/test/parallel/test-tls-socket-destroy.js
new file mode 100644
index 00000000000000..27651f8ec7206a
--- /dev/null
+++ b/test/parallel/test-tls-socket-destroy.js
@@ -0,0 +1,36 @@
+'use strict';
+
+const common = require('../common');
+
+if (!common.hasCrypto) {
+  common.skip('missing crypto');
+  return;
+}
+
+const fs = require('fs');
+const net = require('net');
+const tls = require('tls');
+
+const key = fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem');
+const cert = fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem');
+const secureContext = tls.createSecureContext({ key, cert });
+
+const server = net.createServer(common.mustCall((conn) => {
+  const options = { isServer: true, secureContext, server };
+  const socket = new tls.TLSSocket(conn, options);
+  socket.once('data', common.mustCall(() => {
+    socket._destroySSL();  // Should not crash.
+    server.close();
+  }));
+}));
+
+server.listen(0, function() {
+  const options = {
+    port: this.address().port,
+    rejectUnauthorized: false,
+  };
+  tls.connect(options, function() {
+    this.write('*'.repeat(1 << 20));  // Write more data than fits in a frame.
+    this.on('error', this.destroy);  // Server closes connection on us.
+  });
+});