Skip to content

Commit

Permalink
url: avoid hostname spoofing w/ javascript protocol
Browse files Browse the repository at this point in the history
CVE-2018-12123

Fixes: nodejs-private/security#205
PR-URL: nodejs-private/node-private#145
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
mcollina authored and rvagg committed Nov 27, 2018
1 parent 315ee2e commit d750432
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,13 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
if (slashesDenoteHost || proto || hostPattern.test(rest)) {
var slashes = rest.charCodeAt(0) === CHAR_FORWARD_SLASH &&
rest.charCodeAt(1) === CHAR_FORWARD_SLASH;
if (slashes && !(proto && hostlessProtocol[proto])) {
if (slashes && !(proto && hostlessProtocol[lowerProto])) {
rest = rest.slice(2);
this.slashes = true;
}
}

if (!hostlessProtocol[proto] &&
if (!hostlessProtocol[lowerProto] &&
(slashes || (proto && !slashedProtocol[proto]))) {

// there's a hostname.
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-url-parse-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,39 @@ const parseTests = {
pathname: '/*',
path: '/*',
href: 'https:///*'
},

// The following two URLs are the same, but they differ for
// a capital A: it is important that we verify that the protocol
// is checked in a case-insensitive manner.
'javascript:alert(1);a=\x27@white-listed.com\x27': {
protocol: 'javascript:',
slashes: null,
auth: null,
host: null,
port: null,
hostname: null,
hash: null,
search: null,
query: null,
pathname: "alert(1);a='@white-listed.com'",
path: "alert(1);a='@white-listed.com'",
href: "javascript:alert(1);a='@white-listed.com'"
},

'javAscript:alert(1);a=\x27@white-listed.com\x27': {
protocol: 'javascript:',
slashes: null,
auth: null,
host: null,
port: null,
hostname: null,
hash: null,
search: null,
query: null,
pathname: "alert(1);a='@white-listed.com'",
path: "alert(1);a='@white-listed.com'",
href: "javascript:alert(1);a='@white-listed.com'"
}
};

Expand Down Expand Up @@ -921,3 +954,25 @@ for (const u in parseTests) {
assert.strictEqual(actual, expected,
`format(${u}) == ${u}\nactual:${actual}`);
}

{
const parsed = url.parse('http://nodejs.org/')
.resolveObject('jAvascript:alert(1);a=\x27@white-listed.com\x27');

const expected = Object.assign(new url.Url(), {
protocol: 'javascript:',
slashes: null,
auth: null,
host: null,
port: null,
hostname: null,
hash: null,
search: null,
query: null,
pathname: "alert(1);a='@white-listed.com'",
path: "alert(1);a='@white-listed.com'",
href: "javascript:alert(1);a='@white-listed.com'"
});

assert.deepStrictEqual(parsed, expected);
}

0 comments on commit d750432

Please sign in to comment.