tls: route callback exceptions through error handlers

mcollina · marco-ippolito · commit 25d6799df65c · 2026-01-07T16:38:34.000+01:00
Wrap pskCallback and ALPNCallback invocations in try-catch blocks to route exceptions through owner.destroy() instead of letting them become uncaught exceptions. This prevents remote attackers from crashing TLS servers or causing resource exhaustion. Fixes: https://hackerone.com/reports/3473882 PR-URL: nodejs-private/node-private#782 PR-URL: nodejs-private/node-private#796 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> CVE-ID: CVE-2026-21637
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -235,39 +235,44 @@ function callALPNCallback(protocolsBuffer) {
   const handle = this;
   const socket = handle[owner_symbol];
 
-  const servername = handle.getServername();
+  try {
+    const servername = handle.getServername();
 
-  // Collect all the protocols from the given buffer:
-  const protocols = [];
-  let offset = 0;
-  while (offset < protocolsBuffer.length) {
-    const protocolLen = protocolsBuffer[offset];
-    offset += 1;
+    // Collect all the protocols from the given buffer:
+    const protocols = [];
+    let offset = 0;
+    while (offset < protocolsBuffer.length) {
+      const protocolLen = protocolsBuffer[offset];
+      offset += 1;
 
-    const protocol = protocolsBuffer.slice(offset, offset + protocolLen);
-    offset += protocolLen;
+      const protocol = protocolsBuffer.slice(offset, offset + protocolLen);
+      offset += protocolLen;
 
-    protocols.push(protocol.toString('ascii'));
-  }
+      protocols.push(protocol.toString('ascii'));
+    }
 
-  const selectedProtocol = socket[kALPNCallback]({
-    servername,
-    protocols,
-  });
+    const selectedProtocol = socket[kALPNCallback]({
+      servername,
+      protocols,
+    });
 
-  // Undefined -> all proposed protocols rejected
-  if (selectedProtocol === undefined) return undefined;
+    // Undefined -> all proposed protocols rejected
+    if (selectedProtocol === undefined) return undefined;
 
-  const protocolIndex = protocols.indexOf(selectedProtocol);
-  if (protocolIndex === -1) {
-    throw new ERR_TLS_ALPN_CALLBACK_INVALID_RESULT(selectedProtocol, protocols);
-  }
-  let protocolOffset = 0;
-  for (let i = 0; i < protocolIndex; i++) {
-    protocolOffset += 1 + protocols[i].length;
-  }
+    const protocolIndex = protocols.indexOf(selectedProtocol);
+    if (protocolIndex === -1) {
+      throw new ERR_TLS_ALPN_CALLBACK_INVALID_RESULT(selectedProtocol, protocols);
+    }
+    let protocolOffset = 0;
+    for (let i = 0; i < protocolIndex; i++) {
+      protocolOffset += 1 + protocols[i].length;
+    }
 
-  return protocolOffset;
+    return protocolOffset;
+  } catch (err) {
+    socket.destroy(err);
+    return undefined;
+  }
 }
 
 function requestOCSP(socket, info) {
@@ -374,63 +379,75 @@ function onnewsession(sessionId, session) {
 
 function onPskServerCallback(identity, maxPskLen) {
   const owner = this[owner_symbol];
-  const ret = owner[kPskCallback](owner, identity);
-  if (ret == null)
-    return undefined;
 
-  let psk;
-  if (isArrayBufferView(ret)) {
-    psk = ret;
-  } else {
-    if (typeof ret !== 'object') {
-      throw new ERR_INVALID_ARG_TYPE(
-        'ret',
-        ['Object', 'Buffer', 'TypedArray', 'DataView'],
-        ret,
+  try {
+    const ret = owner[kPskCallback](owner, identity);
+    if (ret == null)
+      return undefined;
+
+    let psk;
+    if (isArrayBufferView(ret)) {
+      psk = ret;
+    } else {
+      if (typeof ret !== 'object') {
+        throw new ERR_INVALID_ARG_TYPE(
+          'ret',
+          ['Object', 'Buffer', 'TypedArray', 'DataView'],
+          ret,
+        );
+      }
+      psk = ret.psk;
+      validateBuffer(psk, 'psk');
+    }
+
+    if (psk.length > maxPskLen) {
+      throw new ERR_INVALID_ARG_VALUE(
+        'psk',
+        psk,
+        `Pre-shared key exceeds ${maxPskLen} bytes`,
       );
     }
-    psk = ret.psk;
-    validateBuffer(psk, 'psk');
-  }
 
-  if (psk.length > maxPskLen) {
-    throw new ERR_INVALID_ARG_VALUE(
-      'psk',
-      psk,
-      `Pre-shared key exceeds ${maxPskLen} bytes`,
-    );
+    return psk;
+  } catch (err) {
+    owner.destroy(err);
+    return undefined;
   }
-
-  return psk;
 }
 
 function onPskClientCallback(hint, maxPskLen, maxIdentityLen) {
   const owner = this[owner_symbol];
-  const ret = owner[kPskCallback](hint);
-  if (ret == null)
-    return undefined;
 
-  validateObject(ret, 'ret');
+  try {
+    const ret = owner[kPskCallback](hint);
+    if (ret == null)
+      return undefined;
+
+    validateObject(ret, 'ret');
+
+    validateBuffer(ret.psk, 'psk');
+    if (ret.psk.length > maxPskLen) {
+      throw new ERR_INVALID_ARG_VALUE(
+        'psk',
+        ret.psk,
+        `Pre-shared key exceeds ${maxPskLen} bytes`,
+      );
+    }
 
-  validateBuffer(ret.psk, 'psk');
-  if (ret.psk.length > maxPskLen) {
-    throw new ERR_INVALID_ARG_VALUE(
-      'psk',
-      ret.psk,
-      `Pre-shared key exceeds ${maxPskLen} bytes`,
-    );
-  }
+    validateString(ret.identity, 'identity');
+    if (Buffer.byteLength(ret.identity) > maxIdentityLen) {
+      throw new ERR_INVALID_ARG_VALUE(
+        'identity',
+        ret.identity,
+        `PSK identity exceeds ${maxIdentityLen} bytes`,
+      );
+    }
 
-  validateString(ret.identity, 'identity');
-  if (Buffer.byteLength(ret.identity) > maxIdentityLen) {
-    throw new ERR_INVALID_ARG_VALUE(
-      'identity',
-      ret.identity,
-      `PSK identity exceeds ${maxIdentityLen} bytes`,
-    );
+    return { psk: ret.psk, identity: ret.identity };
+  } catch (err) {
+    owner.destroy(err);
+    return undefined;
   }
-
-  return { psk: ret.psk, identity: ret.identity };
 }
 
 function onkeylog(line) {
diff --git a/test/parallel/test-tls-alpn-server-client.js b/test/parallel/test-tls-alpn-server-client.js
@@ -253,25 +253,35 @@ function TestALPNCallback() {
 function TestBadALPNCallback() {
   // Server always returns a fixed invalid value:
   const serverOptions = {
+    key: loadPEM('agent2-key'),
+    cert: loadPEM('agent2-cert'),
     ALPNCallback: common.mustCall(() => 'http/5')
   };
 
-  const clientsOptions = [{
-    ALPNProtocols: ['http/1', 'h2'],
-  }];
+  const server = tls.createServer(serverOptions);
 
-  process.once('uncaughtException', common.mustCall((error) => {
+  // Error should be emitted via tlsClientError, not as uncaughtException
+  server.on('tlsClientError', common.mustCall((error, socket) => {
     assert.strictEqual(error.code, 'ERR_TLS_ALPN_CALLBACK_INVALID_RESULT');
+    socket.destroy();
   }));
 
-  runTest(clientsOptions, serverOptions, function(results) {
-    // Callback returns 'http/5' => doesn't match client ALPN => error & reset
-    assert.strictEqual(results[0].server, undefined);
-    const allowedErrors = ['ECONNRESET', 'ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL'];
-    assert.ok(allowedErrors.includes(results[0].client.error.code), `'${results[0].client.error.code}' was not one of ${allowedErrors}.`);
+  server.listen(0, serverIP, common.mustCall(() => {
+    const client = tls.connect({
+      port: server.address().port,
+      host: serverIP,
+      rejectUnauthorized: false,
+      ALPNProtocols: ['http/1', 'h2'],
+    }, common.mustNotCall());
 
-    TestALPNOptionsCallback();
-  });
+    client.on('error', common.mustCall((err) => {
+      // Client gets reset when server handles error via tlsClientError
+      const allowedErrors = ['ECONNRESET', 'ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL'];
+      assert.ok(allowedErrors.includes(err.code), `'${err.code}' was not one of ${allowedErrors}.`);
+      server.close();
+      TestALPNOptionsCallback();
+    }));
+  }));
 }
 
 function TestALPNOptionsCallback() {
diff --git a/test/parallel/test-tls-psk-alpn-callback-exception-handling.js b/test/parallel/test-tls-psk-alpn-callback-exception-handling.js
