GeezerVison is showing github.com — tls: throw on invalid ALPNProtocols instead of aborting · nodejs/node@cd136aa
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/tls.js +29-2Lines changed: 29 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -251,6 +251,10 @@ function convertProtocols(protocols) { | 251 | 251 | const lens = new Array(protocols.length); | 252 | 252 | const buff = Buffer.allocUnsafe(protocols.reduce((p, c, i) => { | 253 | 253 | const len = Buffer.byteLength(c); | | 254 | + if (len === 0) { | | 255 | + throw new ERR_INVALID_ARG_VALUE(`protocols[${i}]`, c, | | 256 | + 'must be a non-empty string'); | | 257 | + } | 254 | 258 | if (len > 255) { | 255 | 259 | throw new ERR_OUT_OF_RANGE('The byte length of the protocol at index ' + | 256 | 260 | `${i} exceeds the maximum length.`, '<= 255', len, true); | @@ -269,18 +273,41 @@ function convertProtocols(protocols) { | 269 | 273 | return buff; | 270 | 274 | } | 271 | 275 |
| | 276 | +function validateALPNBuffer(buffer) { | | 277 | + // Wire format: sequence of <len><proto> where len is 1 byte (1-255) and | | 278 | + // exactly len bytes follow, no trailing bytes, no zero-length entries. | | 279 | + // Empty buffer is allowed and means skip ALPN (same as []). | | 280 | + let offset = 0; | | 281 | + while (offset < buffer.length) { | | 282 | + const len = buffer[offset]; | | 283 | + if (len === 0) { | | 284 | + throw new ERR_INVALID_ARG_VALUE('ALPNProtocols', buffer, | | 285 | + 'must not contain zero-length protocol'); | | 286 | + } | | 287 | + if (offset + 1 + len > buffer.length) { | | 288 | + throw new ERR_INVALID_ARG_VALUE('ALPNProtocols', buffer, | | 289 | + 'contains truncated protocol'); | | 290 | + } | | 291 | + offset += 1 + len; | | 292 | + } | | 293 | +} | | 294 | + | 272 | 295 | exports.convertALPNProtocols = function convertALPNProtocols(protocols, out) { | 273 | 296 | // If protocols is Array - translate it into buffer | 274 | 297 | if (ArrayIsArray(protocols)) { | 275 | 298 | out.ALPNProtocols = convertProtocols(protocols); | 276 | 299 | } else if (isUint8Array(protocols)) { | 277 | 300 | // Copy new buffer not to be modified by user. | 278 | | - out.ALPNProtocols = Buffer.from(protocols); | | 301 | + const buf = Buffer.from(protocols); | | 302 | + validateALPNBuffer(buf); | | 303 | + out.ALPNProtocols = buf; | 279 | 304 | } else if (isArrayBufferView(protocols)) { | 280 | | - out.ALPNProtocols = Buffer.from(protocols.buffer.slice( | | 305 | + const buf = Buffer.from(protocols.buffer.slice( | 281 | 306 | protocols.byteOffset, | 282 | 307 | protocols.byteOffset + protocols.byteLength, | 283 | 308 | )); | | 309 | + validateALPNBuffer(buf); | | 310 | + out.ALPNProtocols = buf; | 284 | 311 | } | 285 | 312 | }; | 286 | 313 |
| |
Collapse file Copy file name to clipboard +85Lines changed: 85 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +'use strict'; | | 2 | +const common = require('../common'); | | 3 | +if (!common.hasCrypto) | | 4 | + common.skip('missing crypto'); | | 5 | + | | 6 | +const assert = require('assert'); | | 7 | +const tls = require('tls'); | | 8 | + | | 9 | +// Array with empty string should throw (zero-length protocol entry) | | 10 | +assert.throws(() => { | | 11 | + const out = {}; | | 12 | + tls.convertALPNProtocols([''], out); | | 13 | +}, { | | 14 | + code: 'ERR_INVALID_ARG_VALUE', | | 15 | +}); | | 16 | + | | 17 | +// Array with empty string mixed | | 18 | +assert.throws(() => { | | 19 | + const out = {}; | | 20 | + tls.convertALPNProtocols(['h2', ''], out); | | 21 | +}, { | | 22 | + code: 'ERR_INVALID_ARG_VALUE', | | 23 | +}); | | 24 | + | | 25 | +// Buffer wire format with leading zero length | | 26 | +assert.throws(() => { | | 27 | + const out = {}; | | 28 | + tls.convertALPNProtocols(Buffer.from([0]), out); | | 29 | +}, { | | 30 | + code: 'ERR_INVALID_ARG_VALUE', | | 31 | +}); | | 32 | + | | 33 | +// Buffer truncated (claims 2 bytes but only 1 follows) | | 34 | +assert.throws(() => { | | 35 | + const out = {}; | | 36 | + tls.convertALPNProtocols(Buffer.from([2, 0x61]), out); | | 37 | +}, { | | 38 | + code: 'ERR_INVALID_ARG_VALUE', | | 39 | +}); | | 40 | + | | 41 | +// Buffer with trailing invalid byte | | 42 | +assert.throws(() => { | | 43 | + const out = {}; | | 44 | + tls.convertALPNProtocols(Buffer.from([1, 0x61, 0x62, 0x62]), out); | | 45 | +}, { | | 46 | + code: 'ERR_INVALID_ARG_VALUE', | | 47 | +}); | | 48 | + | | 49 | +// Empty array means skip ALPN (allowed) | | 50 | +{ | | 51 | + const out = {}; | | 52 | + tls.convertALPNProtocols([], out); | | 53 | + assert.ok(Buffer.isBuffer(out.ALPNProtocols)); | | 54 | + assert.strictEqual(out.ALPNProtocols.length, 0); | | 55 | +} | | 56 | + | | 57 | +// Empty buffer means skip ALPN (allowed; same as []) | | 58 | +{ | | 59 | + const out = {}; | | 60 | + tls.convertALPNProtocols(Buffer.alloc(0), out); | | 61 | + assert.ok(Buffer.isBuffer(out.ALPNProtocols)); | | 62 | + assert.strictEqual(out.ALPNProtocols.length, 0); | | 63 | +} | | 64 | + | | 65 | +// Empty Uint8Array means skip ALPN | | 66 | +{ | | 67 | + const out = {}; | | 68 | + tls.convertALPNProtocols(new Uint8Array(0), out); | | 69 | + assert.ok(Buffer.isBuffer(out.ALPNProtocols)); | | 70 | + assert.strictEqual(out.ALPNProtocols.length, 0); | | 71 | +} | | 72 | + | | 73 | +// Valid inputs should not throw | | 74 | +{ | | 75 | + const out = {}; | | 76 | + tls.convertALPNProtocols(['h2', 'http/1.1'], out); | | 77 | + assert.ok(out.ALPNProtocols.length > 0); | | 78 | +} | | 79 | +{ | | 80 | + const out = {}; | | 81 | + tls.convertALPNProtocols(Buffer.from([ | | 82 | + 2, 0x61, 0x62, 8, 0x68, 0x74, 0x74, 0x70, 0x2f, 0x31, 0x2e, 0x31, | | 83 | + ]), out); | | 84 | + assert.strictEqual(out.ALPNProtocols.length, 12); | | 85 | +} |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-tls-basic-validations.js +8-6Lines changed: 8 additions & 6 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -81,17 +81,19 @@ assert.throws(() => tls.createServer({ ticketKeys: Buffer.alloc(0) }), { | 81 | 81 | }); | 82 | 82 |
| 83 | 83 | { | 84 | | - const buffer = Buffer.from('abcd'); | | 84 | + const buffer = Buffer.from([3, 0x61, 0x62, 0x63]); | 85 | 85 | const out = {}; | 86 | 86 | tls.convertALPNProtocols(buffer, out); | 87 | | - out.ALPNProtocols.write('efgh'); | 88 | | - assert(buffer.equals(Buffer.from('abcd'))); | 89 | | - assert(out.ALPNProtocols.equals(Buffer.from('efgh'))); | | 87 | + out.ALPNProtocols.write('def', 1); | | 88 | + assert(buffer.equals(Buffer.from([3, 0x61, 0x62, 0x63]))); | | 89 | + assert(out.ALPNProtocols.equals(Buffer.from([3, 0x64, 0x65, 0x66]))); | 90 | 90 | } | 91 | 91 |
| 92 | 92 | { | 93 | | - const arrayBufferViewStr = 'abcd'; | 94 | | - const inputBuffer = Buffer.from(arrayBufferViewStr.repeat(8), 'utf8'); | | 93 | + const inputBuffer = Buffer.concat([ | | 94 | + Buffer.from([31]), | | 95 | + Buffer.alloc(31, 0x61), | | 96 | + ]); | 95 | 97 | for (const expectView of common.getArrayBufferViews(inputBuffer)) { | 96 | 98 | const out = {}; | 97 | 99 | const expected = Buffer.from(expectView.buffer.slice(), | |
|