diff --git a/lib/tls.js b/lib/tls.js index 296f6189da1..29420d96084 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; } }; diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index 8ef74aee2d0..ed2ce7ea3ae 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -1702,7 +1702,9 @@ 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()); 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 00000000000..5244e64afc9 --- /dev/null +++ b/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); +}