From d1e7e8822f26e8a49794b757123b51386325b2b0 Mon Sep 17 00:00:00 2001 From: Arnout Kazemier <3rd-Eden@users.noreply.github.com> Date: Wed, 17 Feb 2021 16:17:44 +0100 Subject: [PATCH] [security] More backslash fixes (#197) --- SECURITY.md | 11 +++++++- index.js | 21 ++++++++++---- test/fuzzy.js | 2 ++ test/test.js | 77 ++++++++++++++++++++++++++++++++++++++------------- 4 files changed, 85 insertions(+), 26 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index a1c3d63..31ef5b4 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -33,13 +33,22 @@ acknowledge your responsible disclosure, if you wish. ## History +> Using backslash in the protocol is valid in the browser, while url-parse +> thinks it’s a relative path. An application that validates a url using +> url-parse might pass a malicious link. + +- **Reporter credits** + - CxSCA AppSec team at Checkmarx. + - Twitter: [Yaniv Nizry](https://twitter.com/ynizry) +- Fixed in: 1.5.0 + > The `extractProtocol` method does not return the correct protocol when > provided with unsanitized content which could lead to false positives. - **Reporter credits** - Reported through our security email & Twitter interaction. - Twitter: [@ronperris](https://twitter.com/ronperris) - - Fixed in: 1.4.5 +- Fixed in: 1.4.5 --- diff --git a/index.js b/index.js index 9e58eda..e54575a 100644 --- a/index.js +++ b/index.js @@ -2,8 +2,8 @@ var required = require('requires-port') , qs = require('querystringify') - , slashes = /^[A-Za-z][A-Za-z0-9+-.]*:\/\// - , protocolre = /^([a-z][a-z0-9.+-]*:)?(\/\/)?([\S\s]*)/i + , slashes = /^[A-Za-z][A-Za-z0-9+-.]*:[\\/]+/ + , protocolre = /^([a-z][a-z0-9.+-]*:)?([\\/]{1,})?([\S\s]*)/i , 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]' , left = new RegExp('^'+ whitespace +'+'); @@ -115,11 +115,14 @@ function lolcation(loc) { */ function extractProtocol(address) { address = trimLeft(address); - var match = protocolre.exec(address); + + var match = protocolre.exec(address) + , protocol = match[1] ? match[1].toLowerCase() : '' + , slashes = !!(match[2] && match[2].length >= 2); return { - protocol: match[1] ? match[1].toLowerCase() : '', - slashes: !!match[2], + protocol: protocol, + slashes: slashes, rest: match[3] }; } @@ -280,6 +283,14 @@ function Url(address, location, parser) { url.pathname = resolve(url.pathname, location.pathname); } + // + // Default to a / for pathname if none exists. This normalizes the URL + // to always have a / + // + if (url.pathname.charAt(0) !== '/' && url.hostname) { + url.pathname = '/' + url.pathname; + } + // // We should not add port numbers if they are already the default port number // for a given protocol. As the host also contains the port number we're going diff --git a/test/fuzzy.js b/test/fuzzy.js index f0990d3..6052040 100644 --- a/test/fuzzy.js +++ b/test/fuzzy.js @@ -103,6 +103,8 @@ module.exports = function generate() { , key; spec.protocol = get('protocol'); + spec.slashes = true; + spec.hostname = get('hostname'); spec.pathname = get('pathname'); diff --git a/test/test.js b/test/test.js index 977fa3c..2161761 100644 --- a/test/test.js +++ b/test/test.js @@ -190,9 +190,10 @@ describe('url-parse', function () { , parsed = parse(url); assume(parsed.port).equals(''); + assume(parsed.pathname).equals('/'); assume(parsed.host).equals('example.com'); assume(parsed.hostname).equals('example.com'); - assume(parsed.href).equals('http://example.com'); + assume(parsed.href).equals('http://example.com/'); }); it('understands an / as pathname', function () { @@ -242,7 +243,7 @@ describe('url-parse', function () { assume(parsed.hostname).equals('google.com'); assume(parsed.hash).equals('#what\\is going on'); - parsed = parse('//\\what-is-up.com'); + parsed = parse('http://yolo.com\\what-is-up.com'); assume(parsed.pathname).equals('/what-is-up.com'); }); @@ -250,8 +251,22 @@ describe('url-parse', function () { var url = '////what-is-up.com' , parsed = parse(url); - assume(parsed.host).equals(''); - assume(parsed.hostname).equals(''); + assume(parsed.host).equals('what-is-up.com'); + assume(parsed.href).equals('//what-is-up.com/'); + }); + + it('does not see a slash after the protocol as path', function () { + var url = 'https:\\/github.com/foo/bar' + , parsed = parse(url); + + assume(parsed.host).equals('github.com'); + assume(parsed.hostname).equals('github.com'); + assume(parsed.pathname).equals('/foo/bar'); + + url = 'https:/\/\/\github.com/foo/bar'; + assume(parsed.host).equals('github.com'); + assume(parsed.hostname).equals('github.com'); + assume(parsed.pathname).equals('/foo/bar'); }); describe('origin', function () { @@ -327,32 +342,52 @@ describe('url-parse', function () { it('extracts the right protocol from a url', function () { var testData = [ { - href: 'http://example.com', + href: 'http://example.com/', protocol: 'http:', - pathname: '' + pathname: '/', + slashes: true + }, + { + href: 'ws://example.com/', + protocol: 'ws:', + pathname: '/', + slashes: true + }, + { + href: 'wss://example.com/', + protocol: 'wss:', + pathname: '/', + slashes: true }, { href: 'mailto:test@example.com', pathname: 'test@example.com', - protocol: 'mailto:' + protocol: 'mailto:', + slashes: false }, { href: 'data:text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E', pathname: 'text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E', - protocol: 'data:' + protocol: 'data:', + slashes: false, }, { href: 'sip:alice@atlanta.com', pathname: 'alice@atlanta.com', - protocol: 'sip:' + protocol: 'sip:', + slashes: false, } ]; - var data; + var data, test; for (var i = 0, len = testData.length; i < len; ++i) { - data = parse(testData[i].href); - assume(data.protocol).equals(testData[i].protocol); - assume(data.pathname).equals(testData[i].pathname); + test = testData[i]; + data = parse(test.href); + + assume(data.protocol).equals(test.protocol); + assume(data.pathname).equals(test.pathname); + assume(data.slashes).equals(test.slashes); + assume(data.href).equals(test.href); } }); @@ -391,13 +426,14 @@ describe('url-parse', function () { }); it('parses ipv6 with auth', function () { - var url = 'http://user:password@[3ffe:2a00:100:7031::1]:8080' + var url = 'http://user:password@[3ffe:2a00:100:7031::1]:8080/' , parsed = parse(url); assume(parsed.username).equals('user'); assume(parsed.password).equals('password'); assume(parsed.host).equals('[3ffe:2a00:100:7031::1]:8080'); assume(parsed.hostname).equals('[3ffe:2a00:100:7031::1]'); + assume(parsed.pathname).equals('/'); assume(parsed.href).equals(url); }); @@ -467,7 +503,7 @@ describe('url-parse', function () { assume(data.port).equals(''); assume(data.host).equals('localhost'); - assume(data.href).equals('http://localhost'); + assume(data.href).equals('http://localhost/'); }); it('inherits port numbers for relative urls', function () { @@ -516,7 +552,8 @@ describe('url-parse', function () { }); it('inherits protocol for relative protocols', function () { - var data = parse('//foo.com/foo', parse('http://sub.example.com:808/')); + var lolcation = parse('http://sub.example.com:808/') + , data = parse('//foo.com/foo', lolcation); assume(data.port).equals(''); assume(data.host).equals('foo.com'); @@ -529,13 +566,13 @@ describe('url-parse', function () { assume(data.port).equals(''); assume(data.host).equals('localhost'); - assume(data.href).equals('http://localhost'); + assume(data.href).equals('http://localhost/'); }); it('resolves pathname for relative urls', function () { var data, i = 0; var tests = [ - ['', 'http://foo.com', ''], + ['', 'http://foo.com', '/'], ['', 'http://foo.com/', '/'], ['', 'http://foo.com/a', '/a'], ['a', 'http://foo.com', '/a'], @@ -722,12 +759,12 @@ describe('url-parse', function () { data.set('hash', 'usage'); assume(data.hash).equals('#usage'); - assume(data.href).equals('http://example.com#usage'); + assume(data.href).equals('http://example.com/#usage'); data.set('hash', '#license'); assume(data.hash).equals('#license'); - assume(data.href).equals('http://example.com#license'); + assume(data.href).equals('http://example.com/#license'); }); it('updates the port when updating host', function () {