Skip to content

Commit

Permalink
Handle backslashes like Node.js and Chrome
Browse files Browse the repository at this point in the history
[RFC 2396][] section 2.4.3 puts backslashes (`\`) in the "unwise" list
of characters that aren't allowed in URIs. However, IE, Opera and Chrome
normalize backslashes to slashes (`/`), as noted in [Chromium][].

Since URI.js doesn't do this, it creates possible vulnerabilities. For
example:

```js
var page = URI(window.location.href);
var redirect = URI(page.search(true).redirect_uri);
if (page.domain() === redirect.domain()) {
  window.location = redirect.href();
}
```

This logic will work fine, except when `redirect` has backslashes in the
host, e.g.

```
http://i.xss.com\www.example.org/foo
```

In this case, you'll get:

```js
URI("http://www.example.org").domain();
// example.org
URI("http://i.xss.com\\www.example.org/foo").domain();
// example.org
```

...yet the browsers will redirect you to

```
http://i.xss.com/www.example.org/foo
```

which could be a phishing site.

The supplied change simply replaces all backslashes before the query/hash with slashes. This workaround is also in [Node][Node].

[RFC 2396]: https://www.ietf.org/rfc/rfc2396.txt
[Chromium]: https://code.google.com/p/chromium/issues/detail?id=25916
[Node]: https://github.com/joyent/node/blob/386fd24f49b0e9d1a8a076592a404168faeecc34/lib/url.js#L115-L124
  • Loading branch information
kara-ryli committed Jul 23, 2015
1 parent c51da7e commit 8e13c11
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/URI.js
Expand Up @@ -483,6 +483,12 @@
string = string.substring(0, pos);
}

// Copy chrome, IE, opera backslash-handling behavior.
// Back slashes before the query string get converted to forward slashes
// See: https://github.com/joyent/node/blob/master/lib/url.js
// See: https://code.google.com/p/chromium/issues/detail?id=25916
string = string.replace(/\\/g, '/');

// extract protocol
if (string.substring(0, 2) === '//') {
// relative-scheme
Expand Down
48 changes: 48 additions & 0 deletions test/urls.js
Expand Up @@ -1752,6 +1752,54 @@ var urls = [{
idn: false,
punycode: false
}
}, {
name: 'backslashes',
url: 'http://i.xss.com\\www.example.org/some/directory/file.html?query=string#fragment',
_url: 'http://i.xss.com/www.example.org/some/directory/file.html?query=string#fragment',
parts: {
protocol: 'http',
username: null,
password: null,
hostname: 'i.xss.com',
port: null,
path: '/www.example.org/some/directory/file.html',
query: 'query=string',
fragment: 'fragment'
},
accessors: {
protocol: 'http',
username: '',
password: '',
port: '',
path: '/www.example.org/some/directory/file.html',
query: 'query=string',
fragment: 'fragment',
resource: '/www.example.org/some/directory/file.html?query=string#fragment',
authority: 'i.xss.com',
userinfo: '',
subdomain: 'i',
domain: 'xss.com',
tld: 'com',
directory: '/www.example.org/some/directory',
filename: 'file.html',
suffix: 'html',
hash: '#fragment',
search: '?query=string',
host: 'i.xss.com',
hostname: 'i.xss.com'
},
is: {
urn: false,
url: true,
relative: false,
name: true,
sld: false,
ip: false,
ip4: false,
ip6: false,
idn: false,
punycode: false
}
}
];

0 comments on commit 8e13c11

Please sign in to comment.