Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 65b9c42

Browse filesBrowse files
davisjamtargos
davisjam
authored and
targos
committed
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: #20441 Fixes: #20443 PR-URL: #20445 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent b044256 commit 65b9c42
Copy full SHA for 65b9c42

File tree

Expand file treeCollapse file tree

2 files changed

+35
-5
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+35
-5
lines changed
Open diff view settings
Collapse file

‎lib/dns.js‎

Copy file name to clipboardExpand all lines: lib/dns.js
+10-5Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ function setServers(servers) {
289289
// servers cares won't have any servers available for resolution
290290
const orig = this._handle.getServers();
291291
const newSet = [];
292-
const IPv6RE = /\[(.*)\]/;
292+
const IPv6RE = /^\[([^[\]]*)\]/;
293293
const addrSplitRE = /(^.+?)(?::(\d+))?$/;
294294

295295
servers.forEach((serv) => {
@@ -309,11 +309,16 @@ function setServers(servers) {
309309
}
310310
}
311311

312-
const [, s, p] = serv.match(addrSplitRE);
313-
ipVersion = isIP(s);
312+
// addr::port
313+
const addrSplitMatch = serv.match(addrSplitRE);
314+
if (addrSplitMatch) {
315+
const hostIP = addrSplitMatch[1];
316+
const port = addrSplitMatch[2] || IANA_DNS_PORT;
314317

315-
if (ipVersion !== 0) {
316-
return newSet.push([ipVersion, s, parseInt(p)]);
318+
ipVersion = isIP(hostIP);
319+
if (ipVersion !== 0) {
320+
return newSet.push([ipVersion, hostIP, parseInt(port)]);
321+
}
317322
}
318323

319324
throw new ERR_INVALID_IP_ADDRESS(serv);
Collapse file

‎test/parallel/test-dns.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-dns.js
+25Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,31 @@ assert(existing.length > 0);
6262
]);
6363
}
6464

65+
{
66+
// Various invalidities, all of which should throw a clean error.
67+
const invalidServers = [
68+
' ',
69+
'\n',
70+
'\0',
71+
'1'.repeat(3 * 4),
72+
// Check for REDOS issues.
73+
':'.repeat(100000),
74+
'['.repeat(100000),
75+
'['.repeat(100000) + ']'.repeat(100000) + 'a'
76+
];
77+
invalidServers.forEach((serv) => {
78+
assert.throws(
79+
() => {
80+
dns.setServers([serv]);
81+
},
82+
{
83+
name: 'TypeError [ERR_INVALID_IP_ADDRESS]',
84+
code: 'ERR_INVALID_IP_ADDRESS'
85+
}
86+
);
87+
});
88+
}
89+
6590
const goog = [
6691
'8.8.8.8',
6792
'8.8.4.4',

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.