Skip to content

Commit d1e7e88

Browse files
authored
[security] More backslash fixes (#197)
1 parent d99bf4c commit d1e7e88

File tree

4 files changed

+85
-26
lines changed

4 files changed

+85
-26
lines changed

SECURITY.md

+10-1
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,22 @@ acknowledge your responsible disclosure, if you wish.
3333

3434
## History
3535

36+
> Using backslash in the protocol is valid in the browser, while url-parse
37+
> thinks it’s a relative path. An application that validates a url using
38+
> url-parse might pass a malicious link.
39+
40+
- **Reporter credits**
41+
- CxSCA AppSec team at Checkmarx.
42+
- Twitter: [Yaniv Nizry](https://twitter.com/ynizry)
43+
- Fixed in: 1.5.0
44+
3645
> The `extractProtocol` method does not return the correct protocol when
3746
> provided with unsanitized content which could lead to false positives.
3847
3948
- **Reporter credits**
4049
- Reported through our security email & Twitter interaction.
4150
- Twitter: [@ronperris](https://twitter.com/ronperris)
42-
- Fixed in: 1.4.5
51+
- Fixed in: 1.4.5
4352

4453
---
4554

index.js

+16-5
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
var required = require('requires-port')
44
, qs = require('querystringify')
5-
, slashes = /^[A-Za-z][A-Za-z0-9+-.]*:\/\//
6-
, protocolre = /^([a-z][a-z0-9.+-]*:)?(\/\/)?([\S\s]*)/i
5+
, slashes = /^[A-Za-z][A-Za-z0-9+-.]*:[\\/]+/
6+
, protocolre = /^([a-z][a-z0-9.+-]*:)?([\\/]{1,})?([\S\s]*)/i
77
, whitespace = '[\\x09\\x0A\\x0B\\x0C\\x0D\\x20\\xA0\\u1680\\u180E\\u2000\\u2001\\u2002\\u2003\\u2004\\u2005\\u2006\\u2007\\u2008\\u2009\\u200A\\u202F\\u205F\\u3000\\u2028\\u2029\\uFEFF]'
88
, left = new RegExp('^'+ whitespace +'+');
99

@@ -115,11 +115,14 @@ function lolcation(loc) {
115115
*/
116116
function extractProtocol(address) {
117117
address = trimLeft(address);
118-
var match = protocolre.exec(address);
118+
119+
var match = protocolre.exec(address)
120+
, protocol = match[1] ? match[1].toLowerCase() : ''
121+
, slashes = !!(match[2] && match[2].length >= 2);
119122

120123
return {
121-
protocol: match[1] ? match[1].toLowerCase() : '',
122-
slashes: !!match[2],
124+
protocol: protocol,
125+
slashes: slashes,
123126
rest: match[3]
124127
};
125128
}
@@ -280,6 +283,14 @@ function Url(address, location, parser) {
280283
url.pathname = resolve(url.pathname, location.pathname);
281284
}
282285

286+
//
287+
// Default to a / for pathname if none exists. This normalizes the URL
288+
// to always have a /
289+
//
290+
if (url.pathname.charAt(0) !== '/' && url.hostname) {
291+
url.pathname = '/' + url.pathname;
292+
}
293+
283294
//
284295
// We should not add port numbers if they are already the default port number
285296
// for a given protocol. As the host also contains the port number we're going

test/fuzzy.js

+2
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ module.exports = function generate() {
103103
, key;
104104

105105
spec.protocol = get('protocol');
106+
spec.slashes = true;
107+
106108
spec.hostname = get('hostname');
107109
spec.pathname = get('pathname');
108110

test/test.js

+57-20
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,10 @@ describe('url-parse', function () {
190190
, parsed = parse(url);
191191

192192
assume(parsed.port).equals('');
193+
assume(parsed.pathname).equals('/');
193194
assume(parsed.host).equals('example.com');
194195
assume(parsed.hostname).equals('example.com');
195-
assume(parsed.href).equals('http://example.com');
196+
assume(parsed.href).equals('http://example.com/');
196197
});
197198

198199
it('understands an / as pathname', function () {
@@ -242,16 +243,30 @@ describe('url-parse', function () {
242243
assume(parsed.hostname).equals('google.com');
243244
assume(parsed.hash).equals('#what\\is going on');
244245

245-
parsed = parse('//\\what-is-up.com');
246+
parsed = parse('http://yolo.com\\what-is-up.com');
246247
assume(parsed.pathname).equals('/what-is-up.com');
247248
});
248249

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

253-
assume(parsed.host).equals('');
254-
assume(parsed.hostname).equals('');
254+
assume(parsed.host).equals('what-is-up.com');
255+
assume(parsed.href).equals('//what-is-up.com/');
256+
});
257+
258+
it('does not see a slash after the protocol as path', function () {
259+
var url = 'https:\\/github.com/foo/bar'
260+
, parsed = parse(url);
261+
262+
assume(parsed.host).equals('github.com');
263+
assume(parsed.hostname).equals('github.com');
264+
assume(parsed.pathname).equals('/foo/bar');
265+
266+
url = 'https:/\/\/\github.com/foo/bar';
267+
assume(parsed.host).equals('github.com');
268+
assume(parsed.hostname).equals('github.com');
269+
assume(parsed.pathname).equals('/foo/bar');
255270
});
256271

257272
describe('origin', function () {
@@ -327,32 +342,52 @@ describe('url-parse', function () {
327342
it('extracts the right protocol from a url', function () {
328343
var testData = [
329344
{
330-
href: 'http://example.com',
345+
href: 'http://example.com/',
331346
protocol: 'http:',
332-
pathname: ''
347+
pathname: '/',
348+
slashes: true
349+
},
350+
{
351+
href: 'ws://example.com/',
352+
protocol: 'ws:',
353+
pathname: '/',
354+
slashes: true
355+
},
356+
{
357+
href: 'wss://example.com/',
358+
protocol: 'wss:',
359+
pathname: '/',
360+
slashes: true
333361
},
334362
{
335363
href: 'mailto:test@example.com',
336364
pathname: 'test@example.com',
337-
protocol: 'mailto:'
365+
protocol: 'mailto:',
366+
slashes: false
338367
},
339368
{
340369
href: 'data:text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E',
341370
pathname: 'text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E',
342-
protocol: 'data:'
371+
protocol: 'data:',
372+
slashes: false,
343373
},
344374
{
345375
href: 'sip:alice@atlanta.com',
346376
pathname: 'alice@atlanta.com',
347-
protocol: 'sip:'
377+
protocol: 'sip:',
378+
slashes: false,
348379
}
349380
];
350381

351-
var data;
382+
var data, test;
352383
for (var i = 0, len = testData.length; i < len; ++i) {
353-
data = parse(testData[i].href);
354-
assume(data.protocol).equals(testData[i].protocol);
355-
assume(data.pathname).equals(testData[i].pathname);
384+
test = testData[i];
385+
data = parse(test.href);
386+
387+
assume(data.protocol).equals(test.protocol);
388+
assume(data.pathname).equals(test.pathname);
389+
assume(data.slashes).equals(test.slashes);
390+
assume(data.href).equals(test.href);
356391
}
357392
});
358393

@@ -391,13 +426,14 @@ describe('url-parse', function () {
391426
});
392427

393428
it('parses ipv6 with auth', function () {
394-
var url = 'http://user:password@[3ffe:2a00:100:7031::1]:8080'
429+
var url = 'http://user:password@[3ffe:2a00:100:7031::1]:8080/'
395430
, parsed = parse(url);
396431

397432
assume(parsed.username).equals('user');
398433
assume(parsed.password).equals('password');
399434
assume(parsed.host).equals('[3ffe:2a00:100:7031::1]:8080');
400435
assume(parsed.hostname).equals('[3ffe:2a00:100:7031::1]');
436+
assume(parsed.pathname).equals('/');
401437
assume(parsed.href).equals(url);
402438
});
403439

@@ -467,7 +503,7 @@ describe('url-parse', function () {
467503

468504
assume(data.port).equals('');
469505
assume(data.host).equals('localhost');
470-
assume(data.href).equals('http://localhost');
506+
assume(data.href).equals('http://localhost/');
471507
});
472508

473509
it('inherits port numbers for relative urls', function () {
@@ -516,7 +552,8 @@ describe('url-parse', function () {
516552
});
517553

518554
it('inherits protocol for relative protocols', function () {
519-
var data = parse('//foo.com/foo', parse('http://sub.example.com:808/'));
555+
var lolcation = parse('http://sub.example.com:808/')
556+
, data = parse('//foo.com/foo', lolcation);
520557

521558
assume(data.port).equals('');
522559
assume(data.host).equals('foo.com');
@@ -529,13 +566,13 @@ describe('url-parse', function () {
529566

530567
assume(data.port).equals('');
531568
assume(data.host).equals('localhost');
532-
assume(data.href).equals('http://localhost');
569+
assume(data.href).equals('http://localhost/');
533570
});
534571

535572
it('resolves pathname for relative urls', function () {
536573
var data, i = 0;
537574
var tests = [
538-
['', 'http://foo.com', ''],
575+
['', 'http://foo.com', '/'],
539576
['', 'http://foo.com/', '/'],
540577
['', 'http://foo.com/a', '/a'],
541578
['a', 'http://foo.com', '/a'],
@@ -722,12 +759,12 @@ describe('url-parse', function () {
722759
data.set('hash', 'usage');
723760

724761
assume(data.hash).equals('#usage');
725-
assume(data.href).equals('http://example.com#usage');
762+
assume(data.href).equals('http://example.com/#usage');
726763

727764
data.set('hash', '#license');
728765

729766
assume(data.hash).equals('#license');
730-
assume(data.href).equals('http://example.com#license');
767+
assume(data.href).equals('http://example.com/#license');
731768
});
732769

733770
it('updates the port when updating host', function () {

0 commit comments

Comments
 (0)