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>
This commit is contained in:
Rich Trott 2022-10-16 22:17:55 -07:00 committed by GitHub
parent da8f6182f9
commit a8225dd9ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 27 additions and 25 deletions

View File

@ -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;
const isValid = (code !== CHAR_FORWARD_SLASH &&
code !== CHAR_BACKWARD_SLASH &&
code !== CHAR_HASH &&
code !== CHAR_QUESTION_MARK &&
code !== CHAR_COLON);
// Invalid host character
if (!isValid) {
self.hostname = hostname.slice(0, i);
return `/${hostname.slice(i)}${rest}`;

View File

@ -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) {