diff --git a/lib/tls.js b/lib/tls.js index 296f6189da17..940adef7efaf 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,41 @@ 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. + // 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; } }; 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..2a93cca891ce --- /dev/null +++ b/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); +} diff --git a/test/parallel/test-tls-basic-validations.js b/test/parallel/test-tls-basic-validations.js index 0446b6aef219..d2bbc26b003a 100644 --- a/test/parallel/test-tls-basic-validations.js +++ b/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(),