summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBen Noordhuis <info@bnoordhuis.nl>2017-03-17 15:31:14 +0100
committerMyles Borins <mylesborins@google.com>2017-04-18 20:08:38 -0400
commit54f5258582402644c35693d6fe2bd0a9933b0305 (patch)
tree58a4705d093d4fc7c4f8779dd2086cc690d782af
parent44260806a6fcbf554a9d60f5fd8acaeba071e26f (diff)
downloadnode-new-54f5258582402644c35693d6fe2bd0a9933b0305.tar.gz
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>
-rw-r--r--src/tls_wrap.cc6
-rw-r--r--test/parallel/test-tls-socket-destroy.js36
2 files changed, 42 insertions, 0 deletions
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 056588e495..cab6a75c8c 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -427,6 +427,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 0000000000..27651f8ec7
--- /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.
+ });
+});