From cd73836591e38ae5a31dabe8a779edb5a466189b Mon Sep 17 00:00:00 2001 From: Jamie Davis Date: Mon, 30 Apr 2018 21:27:18 -0400 Subject: [PATCH] dns: 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. Fixes: https://github.com/nodejs/node/issues/20441 --- lib/dns.js | 13 +++++++++---- test/parallel/test-dns.js | 13 +++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/dns.js b/lib/dns.js index 62faf32b690735..cbde0af2c0ac3d 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -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); diff --git a/test/parallel/test-dns.js b/test/parallel/test-dns.js index fb648544b95469..27f6f4a126d096 100644 --- a/test/parallel/test-dns.js +++ b/test/parallel/test-dns.js @@ -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',