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: #38133 fixes: #38135 PR-URL: #38180 Fixes: #38133 Fixes: #38135 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
nodejs · May 30, 2021 · 4ba0659 · 4ba0659
1 parent 1a6ba57
commit 4ba0659
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 2 deletions.
diff --git 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
@@ -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());
+}));