CVE-2018-3774: URL Redirection to Untrusted Site ('Open Redirect') (CWE-601)
value: Incorrect parsing in url-parse <1.4.3 returns wrong hostname which leads to multiple vulnerabilities such as SSRF, Open Redirect, Bypass Authentication Protocol.
CWE description: A web application accepts a user-controlled input that specifies a link to an external site, and uses that link in a Redirect. This simplifies phishing attacks.
CVSS v3 base score: 10
Published: 2018-08-12T22:29Z
Repository: https://github.com/unshiftio/url-parse
Language: JavaScript
Files changed: index.js, test/test.js
Commit date: 2018-07-29 14:26:05 +0200
Commit message: [security] Sanitize paths, hosts before parsing.

Diff: diff --git a/index.js b/index.js
index b88077a..dcc4b41 100644
--- a/index.js
+++ b/index.js
@@ -18,10 +18,13 @@ var required = require('requires-port')
  * 4. `toLowerCase` the resulting value.
  */
 var rules = [
   ['#', 'hash'],                        // Extract from the back.
   ['?', 'query'],                       // Extract from the back.
+  function sanitize(address) {          // Sanitize what is left of the address
+    return address.replace('\\', '/');
+  },
   ['/', 'pathname'],                    // Extract from the back.
   ['@', 'auth', 1],                     // Extract from the front.
   [NaN, 'host', undefined, 1, 1],       // Set left over value.
   [/:(\d+)$/, 'port', undefined, 1],    // RegExp the back.
   [NaN, 'hostname', undefined, 1, 1]    // Set left over.
@@ -45,11 +48,11 @@ var ignore = { hash: 1, query: 1 };
  * encoded in the `pathname` so we can thankfully generate a good "default"
  * location from it so we can generate proper relative URL's again.
  *
  * @param {Object|String} loc Optional default location object.
  * @returns {Object} lolcation object.
- * @api public
+ * @public
  */
 function lolcation(loc) {
   var location = global && global.location || {};
   loc = loc || location;
 
@@ -87,11 +90,11 @@ function lolcation(loc) {
 /**
  * Extract protocol information from a URL with/without double slash ("//").
  *
  * @param {String} address URL we want to extract from.
  * @return {ProtocolExtract} Extracted information.
- * @api private
+ * @private
  */
 function extractProtocol(address) {
   var match = protocolre.exec(address);
 
   return {
@@ -105,11 +108,11 @@ function extractProtocol(address) {
  * Resolve a relative URL pathname against a base URL pathname.
  *
  * @param {String} relative Pathname of the relative URL.
  * @param {String} base Pathname of the base URL.
  * @return {String} Resolved pathname.
- * @api private
+ * @private
  */
 function resolve(relative, base) {
   var path = (base || '/').split('/').slice(0, -1).concat(relative.split('/'))
     , i = path.length
     , last = path[i - 1]
@@ -138,15 +141,18 @@ function resolve(relative, base) {
 /**
  * The actual URL instance. Instead of returning an object we've opted-in to
  * create an actual constructor as it's much more memory efficient and
  * faster and it pleases my OCD.
  *
+ * It is worth noting that we should not use `URL` as class name to prevent
+ * clashes with the global URL instance that got introduced in browsers.
+ *
  * @constructor
  * @param {String} address URL we want to parse.
  * @param {Object|String} location Location defaults for relative paths.
  * @param {Boolean|Function} parser Parser for the query string.
- * @api public
+ * @private
  */
 function Url(address, location, parser) {
   if (!(this instanceof Url)) {
     return new Url(address, location, parser);
   }
@@ -188,14 +194,20 @@ function Url(address, location, parser) {
 
   //
   // When the authority component is absent the URL starts with a path
   // component.
   //
-  if (!extracted.slashes) instructions[2] = [/(.*)/, 'pathname'];
+  if (!extracted.slashes) instructions[3] = [/(.*)/, 'pathname'];
 
   for (; i < instructions.length; i++) {
     instruction = instructions[i];
+
+    if (typeof instruction === 'function') {
+      address = instruction(address);
+      continue;
+    }
+
     parse = instruction[0];
     key = instruction[1];
 
     if (parse !== parse) {
       url[key] = address;
@@ -282,12 +294,12 @@ function Url(address, location, parser) {
  * @param {Mixed} value          The newly assigned value.
  * @param {Boolean|Function} fn  When setting the query, it will be the function
  *                               used to parse the query.
  *                               When setting the protocol, double slash will be
  *                               removed from the final url if it is true.
- * @returns {URL}
- * @api public
+ * @returns {URL} URL instance for chaining.
+ * @public
  */
 function set(part, value, fn) {
   var url = this;
 
   switch (part) {
@@ -368,12 +380,12 @@ function set(part, value, fn) {
 
 /**
  * Transform the properties back in to a valid and full URL string.
  *
  * @param {Function} stringify Optional query stringify function.
- * @returns {String}
- * @api public
+ * @returns {String} Compiled version of the URL.
+ * @public
  */
 function toString(stringify) {
   if (!stringify || 'function' !== typeof stringify) stringify = qs.stringify;
 
   var query
diff --git a/test/test.js b/test/test.js
index 8f47aea..e7a8651 100644
--- a/test/test.js
+++ b/test/test.js
@@ -190,10 +190,32 @@ describe('url-parse', function () {
     assume(parsed.host).equals('x.y.com+a');
     assume(parsed.hostname).equals('x.y.com+a');
     assume(parsed.pathname).equals('/b/c');
   });
 
+  it('ignores \\ in pathnames', function () {
+    var url = 'http://google.com:80\\@yahoo.com/#what\\is going on'
+      , parsed = parse(url);
+
+    assume(parsed.port).equals('');
+    assume(parsed.username).equals('');
+    assume(parsed.password).equals('');
+    assume(parsed.hostname).equals('google.com');
+    assume(parsed.hash).equals('#what\\is going on');
+
+    parsed = parse('//\\what-is-up.com');
+    assume(parsed.pathname).equals('/what-is-up.com');
+  });
+
+  it('correctly ignores multiple slashes //', function () {
+    var url = '////what-is-up.com'
+      , parsed = parse(url);
+
+    assume(parsed.host).equals('');
+    assume(parsed.hostname).equals('');
+  });
+
   describe('origin', function () {
     it('generates an origin property', function () {
       var url = 'http://google.com:80/pathname'
         , parsed = parse(url);
 
@@ -250,10 +272,17 @@ describe('url-parse', function () {
       assume(o.origin).equals('ws://google.com:443');
 
       o = parse('wss://google.com:80/pathname');
       assume(o.origin).equals('wss://google.com:80');
     });
+
+    it('maintains the port number for non-default port numbers', function () {
+      var parsed = parse('http://google.com:8080/pathname');
+
+      assume(parsed.host).equals('http://google.com:8080');
+      assume(parsed.href).equals('http://google.com:8080/pathname');
+    });
   });
 
   describe('protocol', function () {
     it('extracts the right protocol from a url', function () {
       var testData = [

Vulnerable code:  * @api public
 * @api private
 * @api private
 * @api public
  if (!extracted.slashes) instructions[2] = [/(.*)/, 'pathname'];
 * @returns {URL}
 * @api public
 * @returns {String}
 * @api public
Fixed code:   function sanitize(address) {          // Sanitize what is left of the address
    return address.replace('\\', '/');
  },
 * @public
 * @private
 * @private
 * It is worth noting that we should not use `URL` as class name to prevent
 * clashes with the global URL instance that got introduced in browsers.
 *
 * @private
  if (!extracted.slashes) instructions[3] = [/(.*)/, 'pathname'];

    if (typeof instruction === 'function') {
      address = instruction(address);
      continue;
    }

 * @returns {URL} URL instance for chaining.
 * @public
 * @returns {String} Compiled version of the URL.
 * @public
  it('ignores \\ in pathnames', function () {
    var url = 'http://google.com:80\\@yahoo.com/#what\\is going on'
      , parsed = parse(url);

    assume(parsed.port).equals('');
    assume(parsed.username).equals('');
    assume(parsed.password).equals('');
    assume(parsed.hostname).equals('google.com');
    assume(parsed.hash).equals('#what\\is going on');

    parsed = parse('//\\what-is-up.com');
    assume(parsed.pathname).equals('/what-is-up.com');
  });

  it('correctly ignores multiple slashes //', function () {
    var url = '////what-is-up.com'
      , parsed = parse(url);

    assume(parsed.host).equals('');
    assume(parsed.hostname).equals('');
  });


    it('maintains the port number for non-default port numbers', function () {
      var parsed = parse('http://google.com:8080/pathname');

      assume(parsed.host).equals('http://google.com:8080');
      assume(parsed.href).equals('http://google.com:8080/pathname');
    });