diff options
author | Ben Noordhuis <info@bnoordhuis.nl> | 2017-03-17 15:31:14 +0100 |
---|---|---|
committer | Myles Borins <mylesborins@google.com> | 2017-04-18 20:08:38 -0400 |
commit | 54f5258582402644c35693d6fe2bd0a9933b0305 (patch) | |
tree | 58a4705d093d4fc7c4f8779dd2086cc690d782af | |
parent | 44260806a6fcbf554a9d60f5fd8acaeba071e26f (diff) | |
download | node-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.cc | 6 | ||||
-rw-r--r-- | test/parallel/test-tls-socket-destroy.js | 36 |
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. + }); +}); |