Skip to content

Commit eb81b6f

Browse files
Trottdanielleadams
authored andcommitted
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>
1 parent f98a696 commit eb81b6f

File tree

2 files changed

+28
-26
lines changed

2 files changed

+28
-26
lines changed

lib/url.js

+7-20
Original file line numberDiff line numberDiff line change
@@ -128,16 +128,6 @@ const {
128128
CHAR_LEFT_CURLY_BRACKET,
129129
CHAR_RIGHT_CURLY_BRACKET,
130130
CHAR_QUESTION_MARK,
131-
CHAR_LOWERCASE_A,
132-
CHAR_LOWERCASE_Z,
133-
CHAR_UPPERCASE_A,
134-
CHAR_UPPERCASE_Z,
135-
CHAR_DOT,
136-
CHAR_0,
137-
CHAR_9,
138-
CHAR_HYPHEN_MINUS,
139-
CHAR_PLUS,
140-
CHAR_UNDERSCORE,
141131
CHAR_DOUBLE_QUOTE,
142132
CHAR_SINGLE_QUOTE,
143133
CHAR_PERCENT,
@@ -147,6 +137,7 @@ const {
147137
CHAR_GRAVE_ACCENT,
148138
CHAR_VERTICAL_LINE,
149139
CHAR_AT,
140+
CHAR_COLON,
150141
} = require('internal/constants');
151142

152143
function urlParse(url, parseQueryString, slashesDenoteHost) {
@@ -514,16 +505,12 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
514505
function getHostname(self, rest, hostname) {
515506
for (let i = 0; i < hostname.length; ++i) {
516507
const code = hostname.charCodeAt(i);
517-
const isValid = (code >= CHAR_LOWERCASE_A && code <= CHAR_LOWERCASE_Z) ||
518-
code === CHAR_DOT ||
519-
(code >= CHAR_UPPERCASE_A && code <= CHAR_UPPERCASE_Z) ||
520-
(code >= CHAR_0 && code <= CHAR_9) ||
521-
code === CHAR_HYPHEN_MINUS ||
522-
code === CHAR_PLUS ||
523-
code === CHAR_UNDERSCORE ||
524-
code > 127;
525-
526-
// Invalid host character
508+
const isValid = (code !== CHAR_FORWARD_SLASH &&
509+
code !== CHAR_BACKWARD_SLASH &&
510+
code !== CHAR_HASH &&
511+
code !== CHAR_QUESTION_MARK &&
512+
code !== CHAR_COLON);
513+
527514
if (!isValid) {
528515
self.hostname = hostname.slice(0, i);
529516
return `/${hostname.slice(i)}${rest}`;

test/parallel/test-url-parse-format.js

+21-6
Original file line numberDiff line numberDiff line change
@@ -885,15 +885,15 @@ const parseTests = {
885885
protocol: 'https:',
886886
slashes: true,
887887
auth: null,
888-
host: '',
888+
host: '*',
889889
port: null,
890-
hostname: '',
890+
hostname: '*',
891891
hash: null,
892892
search: null,
893893
query: null,
894-
pathname: '/*',
895-
path: '/*',
896-
href: 'https:///*'
894+
pathname: '/',
895+
path: '/',
896+
href: 'https://*/'
897897
},
898898

899899
// The following two URLs are the same, but they differ for a capital A.
@@ -991,7 +991,22 @@ const parseTests = {
991991
pathname: '/',
992992
path: '/',
993993
href: 'http://example.com/'
994-
}
994+
},
995+
996+
'https://evil.com$.example.com': {
997+
protocol: 'https:',
998+
slashes: true,
999+
auth: null,
1000+
host: 'evil.com$.example.com',
1001+
port: null,
1002+
hostname: 'evil.com$.example.com',
1003+
hash: null,
1004+
search: null,
1005+
query: null,
1006+
pathname: '/',
1007+
path: '/',
1008+
href: 'https://evil.com$.example.com/'
1009+
},
9951010
};
9961011

9971012
for (const u in parseTests) {

0 commit comments

Comments
 (0)