From 4e37ac424f4d60a005ed2b29a0a87d2eb8970145 Mon Sep 17 00:00:00 2001 From: sankalpsthakur Date: Thu, 6 Aug 2026 15:10:32 +0530 Subject: [PATCH 1/4] tls: throw on invalid ALPNProtocols instead of aborting Fixes #65069 tls.connect() with malformed ALPNProtocols (empty string, invalid wire format buffers) hits CHECK_EQ(0, SSL_set_alpn_protos(...)) and aborts the process with SIGABRT. Replace the hard abort with THROW_ERR_INVALID_ARG_VALUE so invalid input throws a recoverable JavaScript exception. Assisted-by: Codex Signed-off-by: sankalpsthakur --- src/crypto/crypto_tls.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index 8ef74aee2d0e..b5f6bf708bdc 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -1702,7 +1702,10 @@ void TLSWrap::SetALPNProtocols(const FunctionCallbackInfo& args) { ArrayBufferViewContents protos(args[0].As()); 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( protos.data(), protos.data() + protos.length()); From 9b9eacaefe7a79a47c7808411ecfd3c0d9eb33ad Mon Sep 17 00:00:00 2001 From: sankalpsthakur Date: Thu, 6 Aug 2026 16:17:51 +0530 Subject: [PATCH 2/4] tls: validate ALPNProtocols in JS and cover server case Address review feedback on #65076: move validation closer to call site in convertALPNProtocols so both client and server are covered and fail early. Empty string protocols now throw before reaching OpenSSL; wire-format buffers are validated for truncated/zero-length entries. C++ defense (THROW_ERR_INVALID_ARG_VALUE) remains for safety. Refs: https://github.com/nodejs/node/pull/65076 --- lib/tls.js | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/tls.js b/lib/tls.js index 296f6189da17..7ee4f98f792b 100644 --- a/lib/tls.js +++ b/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 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; } }; From aac57132f6db4fbb52e9e24b156636a52111f44f Mon Sep 17 00:00:00 2001 From: sankalpsthakur Date: Thu, 6 Aug 2026 16:18:04 +0530 Subject: [PATCH 3/4] test: add validation for invalid ALPNProtocols Refs: https://github.com/nodejs/node/issues/65069 PR-URL: https://github.com/nodejs/node/pull/65076 --- .../test-tls-alpn-protocols-validation.js | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 test/parallel/test-tls-alpn-protocols-validation.js diff --git a/test/parallel/test-tls-alpn-protocols-validation.js b/test/parallel/test-tls-alpn-protocols-validation.js new file mode 100644 index 000000000000..2ad500950f51 --- /dev/null +++ b/test/parallel/test-tls-alpn-protocols-validation.js @@ -0,0 +1,65 @@ +'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 byte (len says 1 but 2 bytes remain -> trailing) +assert.throws(() => { + const out = {}; + tls.convertALPNProtocols(Buffer.from([1, 0x61, 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 +assert.doesNotThrow(() => { + const out = {}; + tls.convertALPNProtocols(['h2', 'http/1.1'], out); +}); +assert.doesNotThrow(() => { + const out = {}; + tls.convertALPNProtocols(Buffer.from([2, 0x61, 0x62, 8, 0x68, 0x74, 0x74, 0x70, 0x2f, 0x31, 0x2e, 0x31]), out); +}); From af6b21fc7849be58cb21d558169891a948f44ef2 Mon Sep 17 00:00:00 2001 From: Sankalp Thakur Date: Thu, 6 Aug 2026 18:12:38 +0530 Subject: [PATCH 4/4] fixup: address review: lint and format Signed-off-by: Sankalp Thakur --- lib/tls.js | 8 ++++---- src/crypto/crypto_tls.cc | 3 +-- .../test-tls-alpn-protocols-validation.js | 16 +++++++++------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/tls.js b/lib/tls.js index 7ee4f98f792b..29420d960842 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -255,7 +255,7 @@ function convertProtocols(protocols) { const len = Buffer.byteLength(c); if (len === 0) { throw new ERR_INVALID_ARG_VALUE(`protocols[${i}]`, c, - 'must be a non-empty string'); + 'must be a non-empty string'); } if (len > 255) { throw new ERR_OUT_OF_RANGE('The byte length of the protocol at index ' + @@ -281,17 +281,17 @@ function validateALPNBuffer(buffer) { let offset = 0; if (buffer.length === 0) { throw new ERR_INVALID_ARG_VALUE('ALPNProtocols', buffer, - 'must not be empty'); + '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'); + 'must not contain zero-length protocol'); } if (offset + 1 + len > buffer.length) { throw new ERR_INVALID_ARG_VALUE('ALPNProtocols', buffer, - 'contains truncated protocol'); + 'contains truncated protocol'); } offset += 1 + len; } diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index b5f6bf708bdc..ed2ce7ea3aef 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -1703,8 +1703,7 @@ void TLSWrap::SetALPNProtocols(const FunctionCallbackInfo& args) { SSL* ssl = w->ssl_.get(); if (w->is_client()) { if (SSL_set_alpn_protos(ssl, protos.data(), protos.length()) != 0) { - return THROW_ERR_INVALID_ARG_VALUE( - env, "Invalid ALPNProtocols value"); + return THROW_ERR_INVALID_ARG_VALUE(env, "Invalid ALPNProtocols value"); } } else { w->alpn_protos_ = std::vector( diff --git a/test/parallel/test-tls-alpn-protocols-validation.js b/test/parallel/test-tls-alpn-protocols-validation.js index 2ad500950f51..5244e64afc9f 100644 --- a/test/parallel/test-tls-alpn-protocols-validation.js +++ b/test/parallel/test-tls-alpn-protocols-validation.js @@ -38,10 +38,10 @@ assert.throws(() => { code: 'ERR_INVALID_ARG_VALUE', }); -// Buffer with trailing byte (len says 1 but 2 bytes remain -> trailing) +// Buffer with trailing invalid byte assert.throws(() => { const out = {}; - tls.convertALPNProtocols(Buffer.from([1, 0x61, 0x62]), out); + tls.convertALPNProtocols(Buffer.from([1, 0x61, 0x62, 0x62]), out); }, { code: 'ERR_INVALID_ARG_VALUE', }); @@ -55,11 +55,13 @@ assert.throws(() => { }); // Valid inputs should not throw -assert.doesNotThrow(() => { +{ const out = {}; tls.convertALPNProtocols(['h2', 'http/1.1'], out); -}); -assert.doesNotThrow(() => { +} +{ const out = {}; - tls.convertALPNProtocols(Buffer.from([2, 0x61, 0x62, 8, 0x68, 0x74, 0x74, 0x70, 0x2f, 0x31, 0x2e, 0x31]), out); -}); + tls.convertALPNProtocols(Buffer.from([ + 2, 0x61, 0x62, 8, 0x68, 0x74, 0x74, 0x70, 0x2f, 0x31, 0x2e, 0x31, + ]), out); +}