From c48ebb7ef78cea39bd63b2238ffbb1e97f404ee3 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Wed, 12 Oct 2022 19:28:50 -0700
Subject: [PATCH 1/3] url: improve port validation

If a port is not a number, throw rather than treating the `:` that
delineates the port as part of the path. This is consistent with WHATWG
URL and also mitigates hostname-spoofing.

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                                    | 13 +++++++++----
 test/parallel/test-url-parse-format.js        | 16 ----------------
 test/parallel/test-url-parse-invalid-input.js | 12 ++++++++++++
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/lib/url.js b/lib/url.js
index 328452ba3cb27a..99228c123e5e2c 100644
--- a/lib/url.js
+++ b/lib/url.js
@@ -377,7 +377,7 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
     }
 
     // pull out port.
-    this.parseHost();
+    this.parseHost(url);
 
     // We've indicated that there is a hostname,
     // so even if it's empty, it has to be present.
@@ -392,7 +392,7 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
 
     // validate a little.
     if (!ipv6Hostname) {
-      rest = getHostname(this, rest, hostname);
+      rest = getHostname(this, rest, hostname, url);
     }
 
     if (this.hostname.length > hostnameMaxLen) {
@@ -511,7 +511,7 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
   return this;
 };
 
-function getHostname(self, rest, hostname) {
+function getHostname(self, rest, hostname, url) {
   for (let i = 0; i < hostname.length; ++i) {
     const code = hostname.charCodeAt(i);
     const isValid = (code >= CHAR_LOWERCASE_A && code <= CHAR_LOWERCASE_Z) ||
@@ -526,7 +526,12 @@ function getHostname(self, rest, hostname) {
     // Invalid host character
     if (!isValid) {
       self.hostname = hostname.slice(0, i);
-      return `/${hostname.slice(i)}${rest}`;
+      const leftover = hostname.slice(i);
+      // If leftover starts with :, then it represents an invalid port.
+      if (leftover.startsWith(':')) {
+        throw new ERR_INVALID_URL(url);
+      }
+      return `/${leftover}${rest}`;
     }
   }
   return rest;
diff --git a/test/parallel/test-url-parse-format.js b/test/parallel/test-url-parse-format.js
index 99a6ace23a2fb3..0e6d47382881eb 100644
--- a/test/parallel/test-url-parse-format.js
+++ b/test/parallel/test-url-parse-format.js
@@ -865,22 +865,6 @@ const parseTests = {
     href: 'http://a%0D%22%20%09%0A%3C\'b:b@c/%0D%0Ad/e?f'
   },
 
-  // Git urls used by npm
-  'git+ssh://git@github.com:npm/npm': {
-    protocol: 'git+ssh:',
-    slashes: true,
-    auth: 'git',
-    host: 'github.com',
-    port: null,
-    hostname: 'github.com',
-    hash: null,
-    search: null,
-    query: null,
-    pathname: '/:npm/npm',
-    path: '/:npm/npm',
-    href: 'git+ssh://git@github.com/:npm/npm'
-  },
-
   'https://*': {
     protocol: 'https:',
     slashes: true,
diff --git a/test/parallel/test-url-parse-invalid-input.js b/test/parallel/test-url-parse-invalid-input.js
index 794a756524925b..7ab86380ad7384 100644
--- a/test/parallel/test-url-parse-invalid-input.js
+++ b/test/parallel/test-url-parse-invalid-input.js
@@ -74,3 +74,15 @@ if (common.hasIntl) {
                 (e) => e.code === 'ERR_INVALID_URL',
                 'parsing http://\u00AD/bad.com/');
 }
+
+{
+  const badURLs = [
+    'https://evil.com:.example.com',
+    'git+ssh://git@github.com:npm/npm',
+  ];
+  badURLs.forEach((badURL) => {
+    assert.throws(() => { url.parse(badURL); },
+                  (e) => e.code === 'ERR_INVALID_URL',
+                  `parsing ${badURL}`);
+  });
+}

From 991d54c62347c5dee30e8d89443b389febdd96c4 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Sat, 15 Oct 2022 10:29:09 -0700
Subject: [PATCH 2/3] fixup! url: improve port validation

---
 lib/url.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/url.js b/lib/url.js
index 99228c123e5e2c..da10071416b1e9 100644
--- a/lib/url.js
+++ b/lib/url.js
@@ -377,7 +377,7 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
     }
 
     // pull out port.
-    this.parseHost(url);
+    this.parseHost();
 
     // We've indicated that there is a hostname,
     // so even if it's empty, it has to be present.

From 82bef861f2ac31939ee51983d1707805e551d1fe Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Sat, 15 Oct 2022 10:57:58 -0700
Subject: [PATCH 3/3] fixup! fixup! url: improve port validation

---
 lib/url.js | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/url.js b/lib/url.js
index da10071416b1e9..08a37c21b0b786 100644
--- a/lib/url.js
+++ b/lib/url.js
@@ -525,13 +525,12 @@ function getHostname(self, rest, hostname, url) {
 
     // Invalid host character
     if (!isValid) {
-      self.hostname = hostname.slice(0, i);
-      const leftover = hostname.slice(i);
       // If leftover starts with :, then it represents an invalid port.
-      if (leftover.startsWith(':')) {
+      if (hostname.charCodeAt(i) === 58) {
         throw new ERR_INVALID_URL(url);
       }
-      return `/${leftover}${rest}`;
+      self.hostname = hostname.slice(0, i);
+      return `/${hostname.slice(i)}${rest}`;
     }
   }
   return rest;