tls: throw on invalid ALPNProtocols instead of aborting

lib/tls.js

@@ -253,6 +253,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);
@@ -271,18 +275,44 @@ 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.
+  let offset = 0;
+  if (buffer.length === 0) {
+    throw new ERR_INVALID_ARG_VALUE('ALPNProtocols', buffer,
+                                    'must not be empty');
+  }
+  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;
   }
 };
 

src/crypto/crypto_tls.cc

@@ -1702,7 +1702,9 @@ void TLSWrap::SetALPNProtocols(const FunctionCallbackInfo<Value>& args) {
   ArrayBufferViewContents<uint8_t> protos(args[0].As<ArrayBufferView>());
   SSL* ssl = w->ssl_.get();
   if (w->is_client()) {
-    CHECK_EQ(0, SSL_set_alpn_protos(ssl, protos.data(), protos.length()));
+    if (SSL_set_alpn_protos(ssl, protos.data(), protos.length()) != 0) {
+      return THROW_ERR_INVALID_ARG_VALUE(env, "Invalid ALPNProtocols value");
+    }
   } else {
     w->alpn_protos_ = std::vector<unsigned char>(
         protos.data(), protos.data() + protos.length());

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

@@ -0,0 +1,67 @@
+'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 (client and server paths via convertALPNProtocols)
+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 buffer should throw
+assert.throws(() => {
+  const out = {};
+  tls.convertALPNProtocols(Buffer.alloc(0), out);
+}, {
+  code: 'ERR_INVALID_ARG_VALUE',
+});
+
+// Valid inputs should not throw
+{
+  const out = {};
+  tls.convertALPNProtocols(['h2', 'http/1.1'], out);
+}
+{
+  const out = {};
+  tls.convertALPNProtocols(Buffer.from([
+    2, 0x61, 0x62, 8, 0x68, 0x74, 0x74, 0x70, 0x2f, 0x31, 0x2e, 0x31,
+  ]), out);
+}