From 661e9809bd6dd64270a6f86b37686dc1010ae456 Mon Sep 17 00:00:00 2001 From: Nitzan Uziely Date: Sat, 10 Apr 2021 02:07:56 +0300 Subject: [PATCH] tls: fix session and keylog add listener segfault Fix an issue where adding a session or keylog listener on a tlsSocket after it was destroyed caused a segfault. fixes: https://github.com/nodejs/node/issues/38133 fixes: https://github.com/nodejs/node/issues/38135 PR-URL: https://github.com/nodejs/node/pull/38180 Fixes: https://github.com/nodejs/node/issues/38133 Fixes: https://github.com/nodejs/node/issues/38135 Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- lib/_tls_wrap.js | 8 +++++-- test/parallel/test-tls-tlswrap-segfault-2.js | 22 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-tls-tlswrap-segfault-2.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index e63bcaaf6a7ce9..d357870b46c03a 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -675,7 +675,9 @@ TLSSocket.prototype._init = function(socket, wrap) { if (event !== 'keylog') return; - ssl.enableKeylogCallback(); + // Guard against enableKeylogCallback after destroy + if (!this._handle) return; + this._handle.enableKeylogCallback(); // Remove this listener since it's no longer needed. this.removeListener('newListener', keylogNewListener); @@ -719,7 +721,9 @@ TLSSocket.prototype._init = function(socket, wrap) { if (event !== 'session') return; - ssl.enableSessionCallbacks(); + // Guard against enableSessionCallbacks after destroy + if (!this._handle) return; + this._handle.enableSessionCallbacks(); // Remove this listener since it's no longer needed. this.removeListener('newListener', newListener); diff --git a/test/parallel/test-tls-tlswrap-segfault-2.js b/test/parallel/test-tls-tlswrap-segfault-2.js new file mode 100644 index 00000000000000..d4f3b12b99fdd5 --- /dev/null +++ b/test/parallel/test-tls-tlswrap-segfault-2.js @@ -0,0 +1,22 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// This test ensures that Node.js doesn't incur a segfault while +// adding session or keylog listeners after destroy. +// https://github.com/nodejs/node/issues/38133 +// https://github.com/nodejs/node/issues/38135 + +const tls = require('tls'); +const tlsSocketKeyLog = tls.connect('cause-error'); +tlsSocketKeyLog.on('error', common.mustCall()); +tlsSocketKeyLog.on('close', common.mustCall(() => { + tlsSocketKeyLog.on('keylog', common.mustNotCall()); +})); + +const tlsSocketSession = tls.connect('cause-error-2'); +tlsSocketSession.on('error', common.mustCall()); +tlsSocketSession.on('close', common.mustCall(() => { + tlsSocketSession.on('session', common.mustNotCall()); +}));