url: improve url.parse() compliance with WHATWG URL

Make the url.parse() hostname parsing closer to that of WHATWG URL parsing. This mitigates for cases where hostname spoofing becomes possible if your code checks the hostname using one API but the library you use to send the request uses the other API. Concerns about hostname-spoofing were raised and presented in excellent detail by pyozzi-toss (pyozzi@toss.im/Security-Tech Team in Toss). PR-URL: #45011 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
nodejs · Dec 28, 2022 · e0a916c · e0a916c
1 parent 31d5563
commit e0a916c
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 26 deletions.
diff --git a/lib/url.js b/lib/url.js
@@ -128,16 +128,6 @@ const {
   CHAR_LEFT_CURLY_BRACKET,
   CHAR_RIGHT_CURLY_BRACKET,
   CHAR_QUESTION_MARK,
-  CHAR_LOWERCASE_A,
-  CHAR_LOWERCASE_Z,
-  CHAR_UPPERCASE_A,
-  CHAR_UPPERCASE_Z,
-  CHAR_DOT,
-  CHAR_0,
-  CHAR_9,
-  CHAR_HYPHEN_MINUS,
-  CHAR_PLUS,
-  CHAR_UNDERSCORE,
   CHAR_DOUBLE_QUOTE,
   CHAR_SINGLE_QUOTE,
   CHAR_PERCENT,
@@ -147,6 +137,7 @@ const {
   CHAR_GRAVE_ACCENT,
   CHAR_VERTICAL_LINE,
   CHAR_AT,
+  CHAR_COLON,
 } = require('internal/constants');
 
 function urlParse(url, parseQueryString, slashesDenoteHost) {
@@ -514,16 +505,12 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
 function getHostname(self, rest, hostname) {
   for (let i = 0; i < hostname.length; ++i) {
     const code = hostname.charCodeAt(i);
-    const isValid = (code >= CHAR_LOWERCASE_A && code <= CHAR_LOWERCASE_Z) ||
-                    code === CHAR_DOT ||
-                    (code >= CHAR_UPPERCASE_A && code <= CHAR_UPPERCASE_Z) ||
-                    (code >= CHAR_0 && code <= CHAR_9) ||
-                    code === CHAR_HYPHEN_MINUS ||
-                    code === CHAR_PLUS ||
-                    code === CHAR_UNDERSCORE ||
-                    code > 127;
-
-    // Invalid host character
+    const isValid = (code !== CHAR_FORWARD_SLASH &&
+                     code !== CHAR_BACKWARD_SLASH &&
+                     code !== CHAR_HASH &&
+                     code !== CHAR_QUESTION_MARK &&
+                     code !== CHAR_COLON);
+
     if (!isValid) {
       self.hostname = hostname.slice(0, i);
       return `/${hostname.slice(i)}${rest}`;

diff --git a/test/parallel/test-url-parse-format.js b/test/parallel/test-url-parse-format.js
@@ -885,15 +885,15 @@ const parseTests = {
     protocol: 'https:',
     slashes: true,
     auth: null,
-    host: '',
+    host: '*',
     port: null,
-    hostname: '',
+    hostname: '*',
     hash: null,
     search: null,
     query: null,
-    pathname: '/*',
-    path: '/*',
-    href: 'https:///*'
+    pathname: '/',
+    path: '/',
+    href: 'https://*/'
   },
 
   // The following two URLs are the same, but they differ for a capital A.
@@ -991,7 +991,22 @@ const parseTests = {
     pathname: '/',
     path: '/',
     href: 'http://example.com/'
-  }
+  },
+
+  'https://evil.com$.example.com': {
+    protocol: 'https:',
+    slashes: true,
+    auth: null,
+    host: 'evil.com$.example.com',
+    port: null,
+    hostname: 'evil.com$.example.com',
+    hash: null,
+    search: null,
+    query: null,
+    pathname: '/',
+    path: '/',
+    href: 'https://evil.com$.example.com/'
+  },
 };
 
 for (const u in parseTests) {