From 02cff4a3d3c515699d9670b0426c1b35285d2dd9 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Sun, 16 Oct 2022 22:17:55 -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).

PR-URL: https://github.com/nodejs/node/pull/45011
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
---
 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) {