Skip to content

Commit d750432

Browse files
mcollinarvagg
authored andcommitted
url: avoid hostname spoofing w/ javascript protocol
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>
1 parent 315ee2e commit d750432

File tree

2 files changed

+57
-2
lines changed

2 files changed

+57
-2
lines changed

lib/url.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,13 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
267267
if (slashesDenoteHost || proto || hostPattern.test(rest)) {
268268
var slashes = rest.charCodeAt(0) === CHAR_FORWARD_SLASH &&
269269
rest.charCodeAt(1) === CHAR_FORWARD_SLASH;
270-
if (slashes && !(proto && hostlessProtocol[proto])) {
270+
if (slashes && !(proto && hostlessProtocol[lowerProto])) {
271271
rest = rest.slice(2);
272272
this.slashes = true;
273273
}
274274
}
275275

276-
if (!hostlessProtocol[proto] &&
276+
if (!hostlessProtocol[lowerProto] &&
277277
(slashes || (proto && !slashedProtocol[proto]))) {
278278

279279
// there's a hostname.

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

+55
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,39 @@ const parseTests = {
890890
pathname: '/*',
891891
path: '/*',
892892
href: 'https:///*'
893+
},
894+
895+
// The following two URLs are the same, but they differ for
896+
// a capital A: it is important that we verify that the protocol
897+
// is checked in a case-insensitive manner.
898+
'javascript:alert(1);a=\x27@white-listed.com\x27': {
899+
protocol: 'javascript:',
900+
slashes: null,
901+
auth: null,
902+
host: null,
903+
port: null,
904+
hostname: null,
905+
hash: null,
906+
search: null,
907+
query: null,
908+
pathname: "alert(1);a='@white-listed.com'",
909+
path: "alert(1);a='@white-listed.com'",
910+
href: "javascript:alert(1);a='@white-listed.com'"
911+
},
912+
913+
'javAscript:alert(1);a=\x27@white-listed.com\x27': {
914+
protocol: 'javascript:',
915+
slashes: null,
916+
auth: null,
917+
host: null,
918+
port: null,
919+
hostname: null,
920+
hash: null,
921+
search: null,
922+
query: null,
923+
pathname: "alert(1);a='@white-listed.com'",
924+
path: "alert(1);a='@white-listed.com'",
925+
href: "javascript:alert(1);a='@white-listed.com'"
893926
}
894927
};
895928

@@ -921,3 +954,25 @@ for (const u in parseTests) {
921954
assert.strictEqual(actual, expected,
922955
`format(${u}) == ${u}\nactual:${actual}`);
923956
}
957+
958+
{
959+
const parsed = url.parse('http://nodejs.org/')
960+
.resolveObject('jAvascript:alert(1);a=\x27@white-listed.com\x27');
961+
962+
const expected = Object.assign(new url.Url(), {
963+
protocol: 'javascript:',
964+
slashes: null,
965+
auth: null,
966+
host: null,
967+
port: null,
968+
hostname: null,
969+
hash: null,
970+
search: null,
971+
query: null,
972+
pathname: "alert(1);a='@white-listed.com'",
973+
path: "alert(1);a='@white-listed.com'",
974+
href: "javascript:alert(1);a='@white-listed.com'"
975+
});
976+
977+
assert.deepStrictEqual(parsed, expected);
978+
}

0 commit comments

Comments
 (0)