tls: throw on invalid ALPNProtocols instead of aborting

tls.connect() with malformed ALPNProtocols hit
CHECK_EQ(0, SSL_set_alpn_protos(...)) in the C++ layer and aborted
the process with SIGABRT.

Validate in JS instead, in convertALPNProtocols, so both client and
server fail early with a recoverable ERR_INVALID_ARG_VALUE:

- zero-length string protocols now throw from convertProtocols
- wire-format buffers are checked for zero-length and truncated
  entries
- an empty buffer or array is still accepted and means skip ALPN,
  matching the historical behavior for []

The C++ CHECK_EQ is left unchanged: once JS has validated the input,
a non-zero SSL_set_alpn_protos return is an internal invariant
failure rather than user-facing input.

Fixes: #65069

Signed-off-by: Sankalp Thakur <sankalphimself@gmail.com>

Author: sankalpsthakur

lib/tls.js

@@ -251,6 +251,10 @@ function convertProtocols(protocols) {
   const lens = new Array(protocols.length);
   const buff = Buffer.allocUnsafe(protocols.reduce((p, c, i) => {
     const len = Buffer.byteLength(c);
+    if (len === 0) {
+      throw new ERR_INVALID_ARG_VALUE(`protocols[${i}]`, c,
+                                      'must be a non-empty string');
+    }
     if (len > 255) {
       throw new ERR_OUT_OF_RANGE('The byte length of the protocol at index ' +
         `${i} exceeds the maximum length.`, '<= 255', len, true);
@@ -269,18 +273,41 @@ function convertProtocols(protocols) {
   return buff;
 }
 
+function validateALPNBuffer(buffer) {
+  // Wire format: sequence of <len><proto> where len is 1 byte (1-255) and
+  // exactly len bytes follow, no trailing bytes, no zero-length entries.
+  // Empty buffer is allowed and means skip ALPN (same as []).
+  let offset = 0;
+  while (offset < buffer.length) {
+    const len = buffer[offset];
+    if (len === 0) {
+      throw new ERR_INVALID_ARG_VALUE('ALPNProtocols', buffer,
+                                      'must not contain zero-length protocol');
+    }
+    if (offset + 1 + len > buffer.length) {
+      throw new ERR_INVALID_ARG_VALUE('ALPNProtocols', buffer,
+                                      'contains truncated protocol');
+    }
+    offset += 1 + len;
+  }
+}
+
 exports.convertALPNProtocols = function convertALPNProtocols(protocols, out) {
   // If protocols is Array - translate it into buffer
   if (ArrayIsArray(protocols)) {
     out.ALPNProtocols = convertProtocols(protocols);
   } else if (isUint8Array(protocols)) {
     // Copy new buffer not to be modified by user.
-    out.ALPNProtocols = Buffer.from(protocols);
+    const buf = Buffer.from(protocols);
+    validateALPNBuffer(buf);
+    out.ALPNProtocols = buf;
   } else if (isArrayBufferView(protocols)) {
-    out.ALPNProtocols = Buffer.from(protocols.buffer.slice(
+    const buf = Buffer.from(protocols.buffer.slice(
       protocols.byteOffset,
       protocols.byteOffset + protocols.byteLength,
     ));
+    validateALPNBuffer(buf);
+    out.ALPNProtocols = buf;
   }
 };
 

test/parallel/test-tls-alpn-protocols-validation.js

@@ -0,0 +1,85 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const tls = require('tls');
+
+// Array with empty string should throw (zero-length protocol entry)
+assert.throws(() => {
+  const out = {};
+  tls.convertALPNProtocols([''], out);
+}, {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+
+// Array with empty string mixed
+assert.throws(() => {
+  const out = {};
+  tls.convertALPNProtocols(['h2', ''], out);
+}, {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+
+// Buffer wire format with leading zero length
+assert.throws(() => {
+  const out = {};
+  tls.convertALPNProtocols(Buffer.from([0]), out);
+}, {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+
+// Buffer truncated (claims 2 bytes but only 1 follows)
+assert.throws(() => {
+  const out = {};
+  tls.convertALPNProtocols(Buffer.from([2, 0x61]), out);
+}, {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+
+// Buffer with trailing invalid byte
+assert.throws(() => {
+  const out = {};
+  tls.convertALPNProtocols(Buffer.from([1, 0x61, 0x62, 0x62]), out);
+}, {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+
+// Empty array means skip ALPN (allowed)
+{
+  const out = {};
+  tls.convertALPNProtocols([], out);
+  assert.ok(Buffer.isBuffer(out.ALPNProtocols));
+  assert.strictEqual(out.ALPNProtocols.length, 0);
+}
+
+// Empty buffer means skip ALPN (allowed; same as [])
+{
+  const out = {};
+  tls.convertALPNProtocols(Buffer.alloc(0), out);
+  assert.ok(Buffer.isBuffer(out.ALPNProtocols));
+  assert.strictEqual(out.ALPNProtocols.length, 0);
+}
+
+// Empty Uint8Array means skip ALPN
+{
+  const out = {};
+  tls.convertALPNProtocols(new Uint8Array(0), out);
+  assert.ok(Buffer.isBuffer(out.ALPNProtocols));
+  assert.strictEqual(out.ALPNProtocols.length, 0);
+}
+
+// Valid inputs should not throw
+{
+  const out = {};
+  tls.convertALPNProtocols(['h2', 'http/1.1'], out);
+  assert.ok(out.ALPNProtocols.length > 0);
+}
+{
+  const out = {};
+  tls.convertALPNProtocols(Buffer.from([
+    2, 0x61, 0x62, 8, 0x68, 0x74, 0x74, 0x70, 0x2f, 0x31, 0x2e, 0x31,
+  ]), out);
+  assert.strictEqual(out.ALPNProtocols.length, 12);
+}

test/parallel/test-tls-basic-validations.js

@@ -81,17 +81,19 @@ assert.throws(() => tls.createServer({ ticketKeys: Buffer.alloc(0) }), {
 });
 
 {
-  const buffer = Buffer.from('abcd');
+  const buffer = Buffer.from([3, 0x61, 0x62, 0x63]);
   const out = {};
   tls.convertALPNProtocols(buffer, out);
-  out.ALPNProtocols.write('efgh');
-  assert(buffer.equals(Buffer.from('abcd')));
-  assert(out.ALPNProtocols.equals(Buffer.from('efgh')));
+  out.ALPNProtocols.write('def', 1);
+  assert(buffer.equals(Buffer.from([3, 0x61, 0x62, 0x63])));
+  assert(out.ALPNProtocols.equals(Buffer.from([3, 0x64, 0x65, 0x66])));
 }
 
 {
-  const arrayBufferViewStr = 'abcd';
-  const inputBuffer = Buffer.from(arrayBufferViewStr.repeat(8), 'utf8');
+  const inputBuffer = Buffer.concat([
+    Buffer.from([31]),
+    Buffer.alloc(31, 0x61),
+  ]);
   for (const expectView of common.getArrayBufferViews(inputBuffer)) {
     const out = {};
     const expected = Buffer.from(expectView.buffer.slice(),