Skip to content

Commit

Permalink
tls: throw if protocol too long
Browse files Browse the repository at this point in the history
The convertProtocols() function now throws a range error when the byte
length of a protocol is too long to fit in a Buffer.

Also added a test case in test/parallel/test-tls-basic-validations.js
to cover this.

PR-URL: #23606
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
  • Loading branch information
Andre Jodat-Danbrani authored and MylesBorins committed Dec 26, 2018
1 parent 15d05bb commit 3170cb4
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 4 deletions.
7 changes: 4 additions & 3 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -834,10 +834,11 @@ E('ERR_NO_ICU',
'%s is not supported on Node.js compiled without ICU', TypeError);
E('ERR_NO_LONGER_SUPPORTED', '%s is no longer supported', Error);
E('ERR_OUT_OF_RANGE',
(name, range, value) => {
let msg = `The value of "${name}" is out of range.`;
(str, range, input, replaceDefaultBoolean = false) => {
let msg = replaceDefaultBoolean ? str :
`The value of "${str}" is out of range.`;
if (range !== undefined) msg += ` It must be ${range}.`;
msg += ` Received ${value}`;
msg += ` Received ${input}`;
return msg;
}, RangeError);
E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error);
Expand Down
9 changes: 8 additions & 1 deletion lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@

'use strict';

const { ERR_TLS_CERT_ALTNAME_INVALID } = require('internal/errors').codes;
const {
ERR_TLS_CERT_ALTNAME_INVALID,
ERR_OUT_OF_RANGE
} = require('internal/errors').codes;
const internalUtil = require('internal/util');
const internalTLS = require('internal/tls');
internalUtil.assertCrypto();
Expand Down Expand Up @@ -59,6 +62,10 @@ function convertProtocols(protocols) {
const lens = new Array(protocols.length);
const buff = Buffer.allocUnsafe(protocols.reduce((p, c, i) => {
var len = Buffer.byteLength(c);
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);
}
lens[i] = len;
return p + 1 + len;
}, 0));
Expand Down
13 changes: 13 additions & 0 deletions test/parallel/test-tls-basic-validations.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,16 @@ common.expectsError(
tls.convertNPNProtocols(buffer, out);
assert(out.NPNProtocols.equals(Buffer.from('abcd')));
}

{
const protocols = [(new String('a')).repeat(500)];
const out = {};
common.expectsError(
() => tls.convertALPNProtocols(protocols, out),
{
code: 'ERR_OUT_OF_RANGE',
message: 'The byte length of the protocol at index 0 exceeds the ' +
'maximum length. It must be <= 255. Received 500'
}
);
}

0 comments on commit 3170cb4

Please sign in to comment.