diff --git a/lib/helpers/isURLSameOrigin.js b/lib/helpers/isURLSameOrigin.js index ecf9212365..a9a8e184c7 100644 --- a/lib/helpers/isURLSameOrigin.js +++ b/lib/helpers/isURLSameOrigin.js @@ -22,14 +22,16 @@ module.exports = ( function resolveURL(url) { var href = url; + if (isValidXss(url)) { + throw new Error('URL contains XSS injection attempt'); + } + if (msie) { // IE needs attribute set twice to normalize properties urlParsingNode.setAttribute('href', href); href = urlParsingNode.href; } - isValidXss(url); - urlParsingNode.setAttribute('href', href); // urlParsingNode provides the UrlUtils interface - http://url.spec.whatwg.org/#urlutils diff --git a/lib/helpers/isValidXss.js b/lib/helpers/isValidXss.js index 5783a72015..3545fb5c29 100644 --- a/lib/helpers/isValidXss.js +++ b/lib/helpers/isValidXss.js @@ -1,6 +1,6 @@ 'use strict'; module.exports = function isValidXss(requestURL) { - var regex = RegExp('+.*<\/script>'); - return regex.test(requestURL); + var xssRegex = /(\b)(on\S+)(\s*)=|javascript|(<\s*)(\/*)script/gi; + return xssRegex.test(requestURL); }; diff --git a/test/specs/helpers/isURLSameOrigin.spec.js b/test/specs/helpers/isURLSameOrigin.spec.js index a9d13f5f49..918bc99cee 100644 --- a/test/specs/helpers/isURLSameOrigin.spec.js +++ b/test/specs/helpers/isURLSameOrigin.spec.js @@ -9,7 +9,8 @@ describe('helpers::isURLSameOrigin', function () { expect(isURLSameOrigin('https://github.com/axios/axios')).toEqual(false); }); - it('should detect xss', function () { - expect(isURLSameOrigin('https://github.com/axios/axios?')).toEqual(false) + it('should detect XSS scripts on a same origin request', function () { + expect(() => { isURLSameOrigin('https://github.com/axios/axios?'); }) + .toThrowError(Error, 'URL contains XSS injection attempt') }) }); diff --git a/test/specs/helpers/isValidXss.spec.js b/test/specs/helpers/isValidXss.spec.js new file mode 100644 index 0000000000..b17b686062 --- /dev/null +++ b/test/specs/helpers/isValidXss.spec.js @@ -0,0 +1,24 @@ +var isValidXss = require('../../../lib/helpers/isValidXss'); + +describe('helpers::isValidXss', function () { + it('should detect script tags', function () { + expect(isValidXss("")).toBe(true); + expect(isValidXss("")).toBe(true); + expect(isValidXss("")).toBe(true); + expect(isValidXss("xss")).toBe(true); + expect(isValidXss("")).toBe(true); + expect(isValidXss("onerror=alert('XSS')")).toBe(true); + expect(isValidXss("Click Me")).toBe(true); + }); + + it('should not detect non script tags', function() { + expect(isValidXss(" tags")).toBe(false); + expect(isValidXss("")).toBe(false); + expect(isValidXss(">>> safe <<<")).toBe(false); + expect(isValidXss("<<< safe >>>")).toBe(false); + expect(isValidXss("my script rules")).toBe(false); + expect(isValidXss("")).toBe(false); + expect(isValidXss("

MyTitle

")).toBe(false); + expect(isValidXss("")).toBe(false); + }) +});