dns: improve setServers() errors and performance

Issue 1: make invalid setServers yield uniform error

Behavior:
dns.setServers throws a null pointer dereference on some inputs.
Expected behavior was the more pleasant
  TypeError [ERR_INVALID_IP_ADDRESS] ...

Root cause(s?):
- Dereferencing the result of a regex match without confirming
  that there was a match.
- assuming the capture of an optional group (?)

Solution:
Confirm the match, and handle a missing port cleanly.

Tests: I added tests for various unusual inputs.

Issue 2: revise quadratic regex in setServers

Problem:
The IPv6 regex was quadratic.
On long malicious input the event loop could block.

The security team did not deem it a security risk,
but said a PR was welcome.

Solution:
Revise the regex to a linear-complexity version.

Tests:
I added REDOS tests to the "oddities" section.

Fixes: https://github.com/nodejs/node/issues/20441
Fixes: https://github.com/nodejs/node/issues/20443

PR-URL: https://github.com/nodejs/node/pull/20445
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Jamie Davis 2018-04-30 21:27:18 -04:00 committed by Rich Trott
parent 8551d311bc
commit 872c331a93
2 changed files with 35 additions and 5 deletions

View File

@ -289,7 +289,7 @@ function setServers(servers) {
// servers cares won't have any servers available for resolution
const orig = this._handle.getServers();
const newSet = [];
const IPv6RE = /\[(.*)\]/;
const IPv6RE = /^\[([^[\]]*)\]/;
const addrSplitRE = /(^.+?)(?::(\d+))?$/;
servers.forEach((serv) => {
@ -309,11 +309,16 @@ function setServers(servers) {
}
}
const [, s, p] = serv.match(addrSplitRE);
ipVersion = isIP(s);
// addr::port
const addrSplitMatch = serv.match(addrSplitRE);
if (addrSplitMatch) {
const hostIP = addrSplitMatch[1];
const port = addrSplitMatch[2] || IANA_DNS_PORT;
ipVersion = isIP(hostIP);
if (ipVersion !== 0) {
return newSet.push([ipVersion, s, parseInt(p)]);
return newSet.push([ipVersion, hostIP, parseInt(port)]);
}
}
throw new ERR_INVALID_IP_ADDRESS(serv);

View File

@ -62,6 +62,31 @@ assert(existing.length > 0);
]);
}
{
// Various invalidities, all of which should throw a clean error.
const invalidServers = [
' ',
'\n',
'\0',
'1'.repeat(3 * 4),
// Check for REDOS issues.
':'.repeat(100000),
'['.repeat(100000),
'['.repeat(100000) + ']'.repeat(100000) + 'a'
];
invalidServers.forEach((serv) => {
assert.throws(
() => {
dns.setServers([serv]);
},
{
name: 'TypeError [ERR_INVALID_IP_ADDRESS]',
code: 'ERR_INVALID_IP_ADDRESS'
}
);
});
}
const goog = [
'8.8.8.8',
'8.8.4.4',