From e474cdadfbd24ca86954c15e5a698db9ea45c46f Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Sat, 8 Oct 2022 16:20:01 -0700
Subject: [PATCH] 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).
---
 lib/url.js                             | 27 +++++++-------------------
 test/parallel/test-url-parse-format.js | 27 ++++++++++++++++++++------
 2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/lib/url.js b/lib/url.js
index 328452ba3cb27a..d5462be6332c3e 100644
--- 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
index 99a6ace23a2fb3..0da66f8f99fab0 100644
--- 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) {