fix(xss): Allow src/href attributes on images and links

Relax the XSS sanitization to allow src attributes on images and href attributes on links fix #66
adobe · Jun 5, 2019 · f99d0a5 · f99d0a5
1 parent 193ef93
commit f99d0a5
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 3 deletions.
diff --git a/src/runtime/xss_api.js b/src/runtime/xss_api.js
@@ -142,6 +142,41 @@ function sanitizeURL(url) {
   return '';
 }
 
+/**
+ * Sanitizes the specified attribute in the given array if present.
+ *
+ * @param {string} attribute the attribute to sanitize
+ * @param {*} attribs the attributes array to sanitize from
+ * @returns {object} the sanitized attribute and it's index in the array, or an empty object
+ */
+function sanitizeURLOnAttr(attribute, attribs) {
+  const index = attribs.indexOf(attribute);
+  if (index > -1) {
+    return { index, sanitizedUrl: sanitizeURL(attribs[index + 1]) || null };
+  }
+  return {};
+}
+
+/**
+ * A sanitization policy that validates src/href attributes against the URI scheme.
+ *
+ * @param {string} tagName The name of the tag currently parsed
+ * @param {string[]} attribs An array of attribute names and values
+ * @returns {object} the resulting sanitized attributes
+ */
+function sanitizeURLPolicy(tagName, attribs) {
+  const initial = [].concat(attribs);
+  const result = sanitizer.makeTagPolicy()(tagName, attribs);
+  if (tagName === 'a') {
+    const { index, sanitizedUrl } = sanitizeURLOnAttr('href', initial);
+    result.attribs[index + 1] = sanitizedUrl;
+  } else if (tagName === 'img') {
+    const { index, sanitizedUrl } = sanitizeURLOnAttr('src', initial);
+    result.attribs[index + 1] = sanitizedUrl;
+  }
+  return result;
+}
+
 // function parseValidNumber(input) {
 //   if (NUMBER_PATTERN.test(input)) {
 //     return parseInt(input, 10);
@@ -206,7 +241,7 @@ module.exports = {
    * @returns {String}
    */
   filterHTML(input) {
-    return sanitizer.sanitize(input);
+    return sanitizer.sanitizeWithPolicy(input, sanitizeURLPolicy);
   },
 
   /**

diff --git a/test/runtime_test.js b/test/runtime_test.js
@@ -49,6 +49,7 @@ const GLOBALS = {
   /* eslint-disable no-script-url, no-tabs */
   xss: {
     aTag: '<a href="javascript:alert(0)">XSS Link</a>',
+    aTag2: '<a href="http://www.valid.url">Non XSS Link</a>',
     url1: 'javascript:alert(0)',
     url2: 'javascript://%0Dalert(0)', // js comment & return char
     url3: 'javascript:/*--><script>alert(0);</script>', // js comment & break out of html tag
@@ -61,7 +62,7 @@ const GLOBALS = {
     imgTag2: '<img src="fake.jpg" onerror="alert(0)"/>',
     imgTag3: '<img src=`javascript:alert(0)`/>', // grave accent quotes
     imgTag4: '<img src="java	script:alert(0)"/>', // embedded tab
-    imgTag5: '<img src="java#x0A;script:alert(0)"/>', // embedded encoded tab
+    imgTag5: '<img src="java&#x0A;script:alert(0)"/>', // embedded encoded tab
     scriptTag1: '<script>alert(0);</script>',
     scriptTag2: '<script src="http://do.not.serve/this.js"></script>',
     scriptTag3: '<script src="//do.not.serve/this.js"></script>', // protocol resolution bypass

diff --git a/test/templates/xss.htl b/test/templates/xss.htl
@@ -18,6 +18,7 @@
 <body>
   <ul>
     <li>${xss.aTag}</li>
+    <li>${xss.aTag2}</li>
     <li><a href="${xss.url1}" name="Foo">XSS Link</a></li>
     <li><a href="${xss.url2}">XSS Link</a></li>
     <li><a href="${xss.url3}">XSS Link</a></li>

diff --git a/test/templates/xss.html b/test/templates/xss.html
@@ -18,14 +18,15 @@
 <body>
   <ul>
     <li><a>XSS Link</a></li>
+    <li><a href="http://www.valid.url">Non XSS Link</a></li>
     <li><a name="Foo">XSS Link</a></li>
     <li><a>XSS Link</a></li>
     <li><a>XSS Link</a></li>
     <li><a>XSS Link</a></li>
     <li><a href="#" onclick="alert&#x28;0&#x29;">XSS Link</a></li>
     <li><img></li>
     <li><img src="javascript:alert(0)"/></li>
-    <li><img></li>
+    <li><img src="fake.jpg"></li>
     <li><img></li>
     <li><img></li>
     <li><img></li>