Skip to content

Commit

Permalink
url: throw on NULL in IPv6 hostname
Browse files Browse the repository at this point in the history
Fixes: #39592

PR-URL: #42313
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Trott committed Mar 14, 2022
1 parent 4aae536 commit f524306
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 22 deletions.
52 changes: 30 additions & 22 deletions lib/url.js
Expand Up @@ -177,7 +177,9 @@ function isIpv6Hostname(hostname) {
// as IPv6 by isIpv6Hostname above
//
// [1]: https://url.spec.whatwg.org/#forbidden-host-code-point
const forbiddenHostChars = /[\t\n\r #%/:<>?@[\\\]^|]/;
const forbiddenHostChars = /[\0\t\n\r #%/:<>?@[\\\]^|]/;
// For IPv6, permit '[', ']', and ':'.
const forbiddenHostCharsIpv6 = /[\0\t\n\r #%/<>?@\\^|]/;

Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
validateString(url, 'url');
Expand Down Expand Up @@ -400,27 +402,33 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
this.hostname = this.hostname.toLowerCase();
}

if (!ipv6Hostname && this.hostname !== '') {
// IDNA Support: Returns a punycoded representation of "domain".
// It only converts parts of the domain name that
// have non-ASCII characters, i.e. it doesn't matter if
// you call it with a domain that already is ASCII-only.

// Use lenient mode (`true`) to try to support even non-compliant
// URLs.
this.hostname = toASCII(this.hostname, true);

// Prevent two potential routes of hostname spoofing.
// 1. If this.hostname is empty, it must have become empty due to toASCII
// since we checked this.hostname above.
// 2. If any of forbiddenHostChars appears in this.hostname, it must have
// also gotten in due to toASCII. This is since getHostname would have
// filtered them out otherwise.
// Rather than trying to correct this by moving the non-host part into
// the pathname as we've done in getHostname, throw an exception to
// convey the severity of this issue.
if (this.hostname === '' || forbiddenHostChars.test(this.hostname)) {
throw new ERR_INVALID_URL(url);
if (this.hostname !== '') {
if (ipv6Hostname) {
if (forbiddenHostCharsIpv6.test(this.hostname)) {
throw new ERR_INVALID_URL(url);
}
} else {
// IDNA Support: Returns a punycoded representation of "domain".
// It only converts parts of the domain name that
// have non-ASCII characters, i.e. it doesn't matter if
// you call it with a domain that already is ASCII-only.

// Use lenient mode (`true`) to try to support even non-compliant
// URLs.
this.hostname = toASCII(this.hostname, true);

// Prevent two potential routes of hostname spoofing.
// 1. If this.hostname is empty, it must have become empty due to toASCII
// since we checked this.hostname above.
// 2. If any of forbiddenHostChars appears in this.hostname, it must have
// also gotten in due to toASCII. This is since getHostname would have
// filtered them out otherwise.
// Rather than trying to correct this by moving the non-host part into
// the pathname as we've done in getHostname, throw an exception to
// convey the severity of this issue.
if (this.hostname === '' || forbiddenHostChars.test(this.hostname)) {
throw new ERR_INVALID_URL(url);
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions test/parallel/test-url-parse-invalid-input.js
Expand Up @@ -37,6 +37,10 @@ assert.throws(() => { url.parse('http://%E0%A4%A@fail'); },
return e.code === undefined;
});

assert.throws(() => { url.parse('http://[127.0.0.1\x00c8763]:8000/'); },
{ code: 'ERR_INVALID_URL', input: 'http://[127.0.0.1\x00c8763]:8000/' }
);

if (common.hasIntl) {
// An array of Unicode code points whose Unicode NFKD contains a "bad
// character".
Expand Down

0 comments on commit f524306

Please sign in to comment.