[BUGFIX beta] Sanitize body, link, img attributes

Author: mixonic

packages/ember-htmlbars/lib/attr_nodes.js

@@ -6,18 +6,22 @@
 import QuotedAttrNode from "ember-htmlbars/attr_nodes/quoted";
 import UnquotedAttrNode from "ember-htmlbars/attr_nodes/unquoted";
 import UnquotedNonpropertyAttrNode from "ember-htmlbars/attr_nodes/unquoted_nonproperty";
-import HrefAttrNode from "ember-htmlbars/attr_nodes/href";
+import { badAttributes } from "ember-views/system/sanitize_attribute_value";
+import SanitizedAttrNode from "ember-htmlbars/attr_nodes/sanitized";
 import { create as o_create } from "ember-metal/platform";
 import { normalizeProperty } from "ember-htmlbars/attr_nodes/utils";
 
 var svgNamespaceURI = 'http://www.w3.org/2000/svg';
 
 var unquotedAttrNodeTypes = o_create(null);
 unquotedAttrNodeTypes['class'] = UnquotedNonpropertyAttrNode;
-unquotedAttrNodeTypes['href'] = HrefAttrNode;
 
 var quotedAttrNodeTypes = o_create(null);
-quotedAttrNodeTypes['href'] = HrefAttrNode;
+
+for (var attrName in badAttributes) {
+  unquotedAttrNodeTypes[attrName] = SanitizedAttrNode;
+  quotedAttrNodeTypes[attrName] = SanitizedAttrNode;
+}
 
 export default function attrNodeTypeFor(attrName, element, quoted) {
   var result;

packages/ember-htmlbars/lib/attr_nodes/href.js renamed to packages/ember-htmlbars/lib/attr_nodes/sanitized.js

@@ -8,7 +8,7 @@ import { create as o_create } from "ember-metal/platform";
 import { chainStream, read } from "ember-metal/streams/utils";
 import sanitizeAttributeValue from "ember-views/system/sanitize_attribute_value";
 
-function HrefAttrNode(element, attrName, attrValue, dom) {
+function SanitizedAttrNode(element, attrName, attrValue, dom) {
   var sanitizedValue = chainStream(attrValue, function(){
     var unsafeValue = read(attrValue);
     var safeValue = sanitizeAttributeValue(element, attrName, unsafeValue);
@@ -19,6 +19,6 @@ function HrefAttrNode(element, attrName, attrValue, dom) {
   this.init(element, attrName, sanitizedValue, dom);
 }
 
-HrefAttrNode.prototype = o_create(SimpleAttrNode.prototype);
+SanitizedAttrNode.prototype = o_create(SimpleAttrNode.prototype);
 
-export default HrefAttrNode;
+export default SanitizedAttrNode;

packages/ember-htmlbars/lib/helpers/bind-attr.js

@@ -8,7 +8,8 @@ import Ember from "ember-metal/core"; // Ember.assert
 import { fmt } from "ember-runtime/system/string";
 import QuotedAttrNode from "ember-htmlbars/attr_nodes/quoted";
 import LegacyBindAttrNode from "ember-htmlbars/attr_nodes/legacy_bind";
-import HrefAttrNode from "ember-htmlbars/attr_nodes/href";
+import { badAttributes } from "ember-views/system/sanitize_attribute_value";
+import SanitizedAttrNode from "ember-htmlbars/attr_nodes/sanitized";
 import keys from "ember-metal/keys";
 import helpers from "ember-htmlbars/helpers";
 import { map } from 'ember-metal/enumerable_utils';
@@ -178,8 +179,8 @@ function bindAttrHelper(params, hash, options, env) {
       );
       lazyValue = view.getStream(path);
     }
-    if (attr === 'href') {
-      new HrefAttrNode(element, attr, lazyValue, env.dom);
+    if (badAttributes[attr]) {
+      new SanitizedAttrNode(element, attr, lazyValue, env.dom);
     } else {
       new LegacyBindAttrNode(element, attr, lazyValue, env.dom);
     }

packages/ember-htmlbars/tests/attr_nodes/href_test.js

@@ -1,10 +1,7 @@
-/* jshint scripturl:true */
-
 import EmberView from "ember-views/views/view";
 import run from "ember-metal/run_loop";
 import compile from "ember-template-compiler/system/compile";
 import { equalInnerHTML } from "htmlbars-test-helpers";
-import { SafeString } from "ember-htmlbars/utils/string";
 
 var view;
 
@@ -33,48 +30,4 @@ test("href is set", function() {
                  "attribute is output");
 });
 
-test("href is sanitized when using blacklisted protocol", function() {
-  view = EmberView.create({
-    context: {url: 'javascript://example.com'},
-    template: compile("<a href={{url}}></a>")
-  });
-  appendView(view);
-
-  equalInnerHTML(view.element, '<a href="unsafe:javascript://example.com"></a>',
-                 "attribute is output");
-});
-
-test("href is sanitized when using quoted non-whitelisted protocol", function() {
-  view = EmberView.create({
-    context: {url: 'javascript://example.com'},
-    template: compile("<a href='{{url}}'></a>")
-  });
-  appendView(view);
-
-  equalInnerHTML(view.element, '<a href="unsafe:javascript://example.com"></a>',
-                 "attribute is output");
-});
-
-test("href is not sanitized when using non-whitelisted protocol with a SafeString", function() {
-  view = EmberView.create({
-    context: {url: new SafeString('javascript://example.com')},
-    template: compile("<a href={{url}}></a>")
-  });
-  appendView(view);
-
-  equalInnerHTML(view.element, '<a href="javascript://example.com"></a>',
-                 "attribute is output");
-});
-
-test("href is sanitized when using quoted+concat non-whitelisted protocol", function() {
-  view = EmberView.create({
-    context: {protocol: 'javascript:', path: '//example.com'},
-    template: compile("<a href='{{protocol}}{{path}}'></a>")
-  });
-  appendView(view);
-
-  equalInnerHTML(view.element, '<a href="unsafe:javascript://example.com"></a>',
-                 "attribute is output");
-});
-
 }

packages/ember-htmlbars/tests/attr_nodes/sanitized_test.js

@@ -0,0 +1,98 @@
+/* jshint scripturl:true */
+
+import EmberView from "ember-views/views/view";
+import compile from "ember-htmlbars/system/compile";
+import { SafeString } from "ember-htmlbars/utils/string";
+import { runAppend, runDestroy } from "ember-runtime/tests/utils";
+
+var view;
+
+QUnit.module("ember-htmlbars: sanitized attribute", {
+  teardown: function(){
+    runDestroy(view);
+  }
+});
+
+if (Ember.FEATURES.isEnabled('ember-htmlbars-attribute-syntax')) {
+
+var badTags = [
+  { tag: 'a', attr: 'href',
+    unquotedTemplate: compile("<a href={{url}}></a>"),
+    quotedTemplate: compile("<a href='{{url}}'></a>"),
+    multipartTemplate: compile("<a href='{{protocol}}{{path}}'></a>") },
+  { tag: 'body', attr: 'background',
+    unquotedTemplate: compile("<body background={{url}}></body>"),
+    quotedTemplate: compile("<body background='{{url}}'></body>"),
+    multipartTemplate: compile("<body background='{{protocol}}{{path}}'></body>") },
+  { tag: 'link', attr: 'href',
+    unquotedTemplate: compile("<link href={{url}}>"),
+    quotedTemplate: compile("<link href='{{url}}'>"),
+    multipartTemplate: compile("<link href='{{protocol}}{{path}}'>") },
+  { tag: 'img', attr: 'src',
+    unquotedTemplate: compile("<img src={{url}}>"),
+    quotedTemplate: compile("<img src='{{url}}'>"),
+    multipartTemplate: compile("<img src='{{protocol}}{{path}}'>") },
+];
+
+for (var i=0, l=badTags.length; i<l; i++) {
+  (function(){
+    var subject = badTags[i];
+
+    test(subject.tag +" "+subject.attr+" is sanitized when using blacklisted protocol", function() {
+      view = EmberView.create({
+        context: {url: 'javascript://example.com'},
+        template: subject.unquotedTemplate
+      });
+      runAppend(view);
+
+      equal( view.element.firstChild.getAttribute(subject.attr),
+             "unsafe:javascript://example.com",
+             "attribute is output" );
+    });
+
+    test(subject.tag +" "+subject.attr+" is sanitized when using quoted non-whitelisted protocol", function() {
+      view = EmberView.create({
+        context: {url: 'javascript://example.com'},
+        template: subject.quotedTemplate
+      });
+      runAppend(view);
+
+      equal( view.element.firstChild.getAttribute(subject.attr),
+             "unsafe:javascript://example.com",
+             "attribute is output" );
+    });
+
+    test(subject.tag +" "+subject.attr+" is not sanitized when using non-whitelisted protocol with a SafeString", function() {
+      view = EmberView.create({
+        context: {url: new SafeString('javascript://example.com')},
+        template: subject.unquotedTemplate
+      });
+
+      try {
+        runAppend(view);
+
+        equal( view.element.firstChild.getAttribute(subject.attr),
+               "javascript://example.com",
+               "attribute is output" );
+      } catch(e) {
+        // IE does not allow javascript: to be set on img src
+        ok(true, 'caught exception '+e);
+      }
+    });
+
+    test(subject.tag+" "+subject.attr+" is sanitized when using quoted+concat non-whitelisted protocol", function() {
+      view = EmberView.create({
+        context: {protocol: 'javascript:', path: '//example.com'},
+        template: subject.multipartTemplate
+      });
+      runAppend(view);
+
+      equal( view.element.firstChild.getAttribute(subject.attr),
+             "unsafe:javascript://example.com",
+             "attribute is output" );
+    });
+
+  })(); //jshint ignore:line
+}
+
+}

packages/ember-htmlbars/tests/helpers/bind_attr_test.js

@@ -15,8 +15,6 @@ import { equalInnerHTML } from "htmlbars-test-helpers";
 
 import helpers from "ember-htmlbars/helpers";
 import compile from "ember-template-compiler/system/compile";
-import { SafeString } from "ember-htmlbars/utils/string";
-
 var view;
 
 var originalLookup = Ember.lookup;
@@ -530,52 +528,3 @@ test("property before didInsertElement", function() {
   runAppend(view);
   equal(matchingElement.length, 1, 'element is in the DOM when didInsertElement');
 });
-
-test("XSS - should not bind unsafe href values", function() {
-  /* jshint scripturl:true */
-
-  view = EmberView.create({
-    template: compile('<a {{bind-attr href=view.badValue}}>wat</a>'),
-    badValue: "javascript:(function() { window.bar = 'NOOOOOOOOO!!!!!!!';})();"
-  });
-
-  runAppend(view);
-
-  var element = view.$('a')[0];
-
-  notEqual(element.protocol, 'javascript:');
-});
-
-test("XSS - should not bind unsafe href values on rerender", function() {
-  /* jshint scripturl:true */
-
-  view = EmberView.create({
-    template: compile('<a {{bind-attr href=view.badValue}}>wat</a>'),
-    badValue: "/sunshine/and/rainbows"
-  });
-
-  runAppend(view);
-
-  var element = view.$('a')[0];
-
-  notEqual(element.protocol, 'javascript:', 'precond - badValue is normal');
-
-  run(view, 'set', 'badValue', 'javascript:alert("XSS")');
-
-  notEqual(element.protocol, 'javascript:');
-});
-
-test("should bind unsafe href values if they are SafeString", function() {
-  /* jshint scripturl:true */
-
-  view = EmberView.create({
-    template: compile('<a {{bind-attr href=view.badValue}}>wat</a>'),
-    badValue: new SafeString("javascript:(function() { window.bar = 'NOOOOOOOOO!!!!!!!';})();")
-  });
-
-  runAppend(view);
-
-  var element = view.$('a')[0];
-
-  equal(element.protocol, 'javascript:');
-});

packages/ember-htmlbars/tests/helpers/sanitized_bind_attr_test.js

@@ -0,0 +1,84 @@
+/* jshint scripturl:true */
+
+import EmberView from "ember-views/views/view";
+import compile from "ember-htmlbars/system/compile";
+import run from "ember-metal/run_loop";
+import { SafeString } from "ember-htmlbars/utils/string";
+import { runAppend, runDestroy } from "ember-runtime/tests/utils";
+
+var view;
+
+QUnit.module("ember-htmlbars: sanitized attribute", {
+  teardown: function(){
+    runDestroy(view);
+  }
+});
+
+var badTags = [
+  { tag: 'a', attr: 'href',
+    template: compile('<a {{bind-attr href=view.badValue}}></a>') },
+  { tag: 'body', attr: 'background',
+    template: compile('<body {{bind-attr background=view.badValue}}></body>') },
+  { tag: 'link', attr: 'href',
+    template: compile('<link {{bind-attr href=view.badValue}}>') },
+  { tag: 'img', attr: 'src',
+    template: compile('<img {{bind-attr src=view.badValue}}>') }
+];
+
+for (var i=0, l=badTags.length; i<l; i++) {
+  (function(){
+    var tagName = badTags[i].tag;
+    var attr = badTags[i].attr;
+    var template = badTags[i].template;
+
+    test("XSS - should not bind unsafe "+tagName+" "+attr+" values", function() {
+      view = EmberView.create({
+        template: template,
+        badValue: "javascript:alert('XSS')"
+      });
+
+      runAppend(view);
+
+      equal( view.element.firstChild.getAttribute(attr),
+             "unsafe:javascript:alert('XSS')",
+             "attribute is output" );
+    });
+
+    test("XSS - should not bind unsafe "+tagName+" "+attr+" values on rerender", function() {
+      view = EmberView.create({
+        template: template,
+        badValue: "/sunshine/and/rainbows"
+      });
+
+      runAppend(view);
+
+      equal( view.element.firstChild.getAttribute(attr),
+             "/sunshine/and/rainbows",
+             "attribute is output" );
+
+      run(view, 'set', 'badValue', "javascript:alert('XSS')");
+
+      equal( view.element.firstChild.getAttribute(attr),
+             "unsafe:javascript:alert('XSS')",
+             "attribute is output" );
+    });
+
+    test("should bind unsafe "+tagName+" "+attr+" values if they are SafeString", function() {
+      view = EmberView.create({
+        template: template,
+        badValue: new SafeString("javascript:alert('XSS')")
+      });
+
+      try {
+        runAppend(view);
+
+        equal( view.element.firstChild.getAttribute(attr),
+               "javascript:alert('XSS')",
+               "attribute is output" );
+      } catch(e) {
+        // IE does not allow javascript: to be set on img src
+        ok(true, 'caught exception '+e);
+      }
+    });
+  })(); //jshint ignore:line
+}

packages/ember-views/lib/system/sanitize_attribute_value.js

@@ -6,6 +6,19 @@ var badProtocols = {
   'vbscript:': true
 };
 
+var badTags = {
+  'A': true,
+  'BODY': true,
+  'LINK': true,
+  'IMG': true
+};
+
+export var badAttributes = {
+  'href': true,
+  'src': true,
+  'background': true
+};
+
 export default function sanitizeAttributeValue(element, attribute, value) {
   var tagName;
 
@@ -23,7 +36,7 @@ export default function sanitizeAttributeValue(element, attribute, value) {
     return value.toHTML();
   }
 
-  if ((tagName === null || tagName === 'A') && attribute === 'href') {
+  if ((tagName === null || badTags[tagName]) && badAttributes[attribute]) {
     parsingNode.href = value;
 
     if (badProtocols[parsingNode.protocol] === true) {