Fix to prevent XSS, throw an error when the URL contains a JS script (#…

…2464) * Fixes issue where XSS scripts attacks were possible via the URL * Fix error * Move throwing error up * Add specs and make regex cover more xss cases
axios · Oct 16, 2019 · 29da6b2 · 29da6b2 · felipecwb · Jan 14, 2020
1 parent ee60ee3
commit 29da6b2
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 6 deletions.
diff --git 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
@@ -1,6 +1,6 @@
 'use strict';
 
 module.exports = function isValidXss(requestURL) {
-  var regex = RegExp('<script+.*>+.*<\/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
@@ -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?<script>alert("hello")</script>')).toEqual(false)
+  it('should detect XSS scripts on a same origin request', function () {
+    expect(() => { isURLSameOrigin('https://github.com/axios/axios?<script>alert("hello")</script>'); })
+      .toThrowError(Error, 'URL contains XSS injection attempt')
   })
 });
diff --git a/test/specs/helpers/isValidXss.spec.js 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("<script/xss>alert('blah')</script/xss>")).toBe(true);
+    expect(isValidXss("<SCRIPT>alert('getting your password')</SCRIPT>")).toBe(true);
+    expect(isValidXss("<script src='http://xssinjections.com/inject.js'>xss</script>")).toBe(true);
+    expect(isValidXss("<img src='/' onerror='javascript:alert('xss')'>xss</script>")).toBe(true);
+    expect(isValidXss("<script>console.log('XSS')</script>")).toBe(true);
+    expect(isValidXss("onerror=alert('XSS')")).toBe(true);
+    expect(isValidXss("<a onclick='alert('XSS')'>Click Me</a>")).toBe(true);
+  });
+
+  it('should not detect non script tags', function() {
+    expect(isValidXss("<safe> tags")).toBe(false);
+    expect(isValidXss("<safetag>")).toBe(false);
+    expect(isValidXss(">>> safe <<<")).toBe(false);
+    expect(isValidXss("<<< safe >>>")).toBe(false);
+    expect(isValidXss("my script rules")).toBe(false);
+    expect(isValidXss("<a notonlistener='nomatch'>")).toBe(false);
+    expect(isValidXss("<h2>MyTitle</h2>")).toBe(false);
+    expect(isValidXss("<img src='#'/>")).toBe(false);
+  })
+});