Skip to content

Commit

Permalink
dns: make invalid setServers yield uniform error
Browse files Browse the repository at this point in the history
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.

Fixes: nodejs#20441
  • Loading branch information
davisjam committed May 1, 2018
1 parent e954aa0 commit cd73836
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
13 changes: 9 additions & 4 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

if (ipVersion !== 0) {
return newSet.push([ipVersion, s, parseInt(p)]);
ipVersion = isIP(hostIP);
if (ipVersion !== 0) {
return newSet.push([ipVersion, hostIP, parseInt(port)]);
}
}

throw new ERR_INVALID_IP_ADDRESS(serv);
Expand Down
13 changes: 13 additions & 0 deletions test/parallel/test-dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,19 @@ assert(existing.length > 0);
]);
}

{
// Various invalidities, all of which should throw a clean error.
const invalidServers = [
' ',
'\n',
'\0',
'1'.repeat(3 * 4)
];
invalidServers.forEach((serv) => {
assert.throws(() => dns.setServers([serv]), /TypeError.*ERR_INVALID_IP_ADDRESS/, `Unexpected error thrown for ${serv}`);
});
}

const goog = [
'8.8.8.8',
'8.8.4.4',
Expand Down

0 comments on commit cd73836

Please sign in to comment.