Skip to content

Commit 724142b

Browse files
rwjbluerwjblue
committed
[BUGFIX beta] Sanitize Href attribute values.
1 parent a31d8d2 commit 724142b

File tree

12 files changed

+278
-3
lines changed

12 files changed

+278
-3
lines changed

‎packages/ember-handlebars/lib/helpers/bind_attr.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { forEach } from "ember-metal/array";
1313
import View from "ember-views/views/view";
1414
import keys from "ember-metal/keys";
1515

16+
import sanitizeAttributeValue from "ember-views/system/sanitize_attribute_value";
17+
1618
var helpers = EmberHandlebars.helpers;
1719
var SafeString = EmberHandlebars.SafeString;
1820

@@ -178,6 +180,7 @@ function bindAttrHelper(options) {
178180

179181
var lazyValue = view.getStream(path);
180182
var value = lazyValue.value();
183+
value = sanitizeAttributeValue(null, attr, value);
181184
var type = typeOf(value);
182185

183186
Ember.assert(fmt("Attributes must be numbers, strings or booleans, not %@", [value]),

‎packages/ember-htmlbars/lib/attr_nodes.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,18 @@
66
import QuotedAttrNode from "ember-htmlbars/attr_nodes/quoted";
77
import UnquotedAttrNode from "ember-htmlbars/attr_nodes/unquoted";
88
import UnquotedNonpropertyAttrNode from "ember-htmlbars/attr_nodes/unquoted_nonproperty";
9+
import HrefAttrNode from "ember-htmlbars/attr_nodes/href";
910
import { create as o_create } from "ember-metal/platform";
1011
import { normalizeProperty } from "ember-htmlbars/attr_nodes/utils";
1112

1213
var svgNamespaceURI = 'http://www.w3.org/2000/svg';
1314

1415
var unquotedAttrNodeTypes = o_create(null);
1516
unquotedAttrNodeTypes['class'] = UnquotedNonpropertyAttrNode;
17+
unquotedAttrNodeTypes['href'] = HrefAttrNode;
1618

1719
var quotedAttrNodeTypes = o_create(null);
20+
quotedAttrNodeTypes['href'] = HrefAttrNode;
1821

1922
export default function attrNodeTypeFor(attrName, element, quoted) {
2023
var result;

‎packages/ember-htmlbars/lib/attr_nodes/href.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
@module ember
3+
@submodule ember-htmlbars
4+
*/
5+
6+
import SimpleAttrNode from "./simple";
7+
import { create as o_create } from "ember-metal/platform";
8+
import { chainStream, read } from "ember-metal/streams/utils";
9+
import sanitizeAttributeValue from "ember-views/system/sanitize_attribute_value";
10+
11+
function HrefAttrNode(element, attrName, attrValue, dom) {
12+
var sanitizedValue = chainStream(attrValue, function(){
13+
var unsafeValue = read(attrValue);
14+
var safeValue = sanitizeAttributeValue(element, attrName, unsafeValue);
15+
16+
return safeValue;
17+
});
18+
19+
this.init(element, attrName, sanitizedValue, dom);
20+
}
21+
22+
HrefAttrNode.prototype = o_create(SimpleAttrNode.prototype);
23+
24+
export default HrefAttrNode;

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import Ember from "ember-metal/core"; // Ember.assert
88
import { fmt } from "ember-runtime/system/string";
99
import QuotedAttrNode from "ember-htmlbars/attr_nodes/quoted";
1010
import LegacyBindAttrNode from "ember-htmlbars/attr_nodes/legacy_bind";
11+
import HrefAttrNode from "ember-htmlbars/attr_nodes/href";
1112
import keys from "ember-metal/keys";
1213
import helpers from "ember-htmlbars/helpers";
1314
import { map } from 'ember-metal/enumerable_utils';
@@ -177,7 +178,11 @@ function bindAttrHelper(params, hash, options, env) {
177178
);
178179
lazyValue = view.getStream(path);
179180
}
180-
new LegacyBindAttrNode(element, attr, lazyValue, env.dom);
181+
if (attr === 'href') {
182+
new HrefAttrNode(element, attr, lazyValue, env.dom);
183+
} else {
184+
new LegacyBindAttrNode(element, attr, lazyValue, env.dom);
185+
}
181186
}
182187
}
183188

‎packages/ember-htmlbars/lib/hooks/attribute.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import attrNodeTypeFor from "ember-htmlbars/attr_nodes";
77
import EmberError from "ember-metal/error";
88
import { isStream } from "ember-metal/streams/utils";
9+
import sanitizeAttributeValue from "ember-views/system/sanitize_attribute_value";
910

1011
var boundAttributesEnabled = false;
1112

@@ -21,7 +22,8 @@ export default function attribute(element, attrName, quoted, view, attrValue, op
2122
if (isStream(attrValue)) {
2223
throw new EmberError('Bound attributes are not yet supported in Ember.js');
2324
} else {
24-
env.dom.setAttribute(element, attrName, attrValue);
25+
var sanitizedValue = sanitizeAttributeValue(element, attrName, attrValue);
26+
env.dom.setAttribute(element, attrName, sanitizedValue);
2527
}
2628
}
2729
}

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
/* jshint scripturl:true */
2+
13
import EmberView from "ember-views/views/view";
24
import run from "ember-metal/run_loop";
35
import compile from "ember-template-compiler/system/compile";
46
import { equalInnerHTML } from "htmlbars-test-helpers";
7+
import { SafeString } from "ember-htmlbars/utils/string";
58

69
var view;
710

@@ -30,4 +33,48 @@ test("href is set", function() {
3033
"attribute is output");
3134
});
3235

36+
test("href is sanitized when using blacklisted protocol", function() {
37+
view = EmberView.create({
38+
context: {url: 'javascript://example.com'},
39+
template: compile("<a href={{url}}></a>")
40+
});
41+
appendView(view);
42+
43+
equalInnerHTML(view.element, '<a href="unsafe:javascript://example.com"></a>',
44+
"attribute is output");
45+
});
46+
47+
test("href is sanitized when using quoted non-whitelisted protocol", function() {
48+
view = EmberView.create({
49+
context: {url: 'javascript://example.com'},
50+
template: compile("<a href='{{url}}'></a>")
51+
});
52+
appendView(view);
53+
54+
equalInnerHTML(view.element, '<a href="unsafe:javascript://example.com"></a>',
55+
"attribute is output");
56+
});
57+
58+
test("href is not sanitized when using non-whitelisted protocol with a SafeString", function() {
59+
view = EmberView.create({
60+
context: {url: new SafeString('javascript://example.com')},
61+
template: compile("<a href={{url}}></a>")
62+
});
63+
appendView(view);
64+
65+
equalInnerHTML(view.element, '<a href="javascript://example.com"></a>',
66+
"attribute is output");
67+
});
68+
69+
test("href is sanitized when using quoted+concat non-whitelisted protocol", function() {
70+
view = EmberView.create({
71+
context: {protocol: 'javascript:', path: '//example.com'},
72+
template: compile("<a href='{{protocol}}{{path}}'></a>")
73+
});
74+
appendView(view);
75+
76+
equalInnerHTML(view.element, '<a href="unsafe:javascript://example.com"></a>',
77+
"attribute is output");
78+
});
79+
3380
}

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { equalInnerHTML } from "htmlbars-test-helpers";
1515

1616
import helpers from "ember-htmlbars/helpers";
1717
import compile from "ember-template-compiler/system/compile";
18+
import { SafeString } from "ember-htmlbars/utils/string";
1819

1920
var view;
2021

@@ -529,3 +530,52 @@ test("property before didInsertElement", function() {
529530
runAppend(view);
530531
equal(matchingElement.length, 1, 'element is in the DOM when didInsertElement');
531532
});
533+
534+
test("XSS - should not bind unsafe href values", function() {
535+
/* jshint scripturl:true */
536+
537+
view = EmberView.create({
538+
template: compile('<a {{bind-attr href=view.badValue}}>wat</a>'),
539+
badValue: "javascript:(function() { window.bar = 'NOOOOOOOOO!!!!!!!';})();"
540+
});
541+
542+
runAppend(view);
543+
544+
var element = view.$('a')[0];
545+
546+
notEqual(element.protocol, 'javascript:');
547+
});
548+
549+
test("XSS - should not bind unsafe href values on rerender", function() {
550+
/* jshint scripturl:true */
551+
552+
view = EmberView.create({
553+
template: compile('<a {{bind-attr href=view.badValue}}>wat</a>'),
554+
badValue: "/sunshine/and/rainbows"
555+
});
556+
557+
runAppend(view);
558+
559+
var element = view.$('a')[0];
560+
561+
notEqual(element.protocol, 'javascript:', 'precond - badValue is normal');
562+
563+
run(view, 'set', 'badValue', 'javascript:alert("XSS")');
564+
565+
notEqual(element.protocol, 'javascript:');
566+
});
567+
568+
test("should bind unsafe href values if they are SafeString", function() {
569+
/* jshint scripturl:true */
570+
571+
view = EmberView.create({
572+
template: compile('<a {{bind-attr href=view.badValue}}>wat</a>'),
573+
badValue: new SafeString("javascript:(function() { window.bar = 'NOOOOOOOOO!!!!!!!';})();")
574+
});
575+
576+
runAppend(view);
577+
578+
var element = view.$('a')[0];
579+
580+
equal(element.protocol, 'javascript:');
581+
});

‎packages/ember-htmlbars/tests/helpers/unbound_test.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,36 @@ test('it should throw the helper missing error if multiple properties are provid
6969
}, EmberError);
7070
});
7171

72+
test('should property escape unsafe hrefs', function() {
73+
/* jshint scripturl:true */
74+
75+
expect(3);
76+
77+
runDestroy(view);
78+
79+
view = EmberView.create({
80+
template: compile('<ul>{{#each person in view.people}}<a href="{{unbound person.url}}">{{person.name}}</a>{{/each}}</ul>'),
81+
people: A([{
82+
name: 'Bob',
83+
url: 'javascript:bob-is-cool'
84+
}, {
85+
name: 'James',
86+
url: 'vbscript:james-is-cool'
87+
}, {
88+
name: 'Richard',
89+
url: 'javascript:richard-is-cool'
90+
}])
91+
});
92+
93+
runAppend(view);
94+
95+
var links = view.$('a');
96+
for (var i = 0, l = links.length; i < l; i++) {
97+
var link = links[i];
98+
equal(link.protocol, 'unsafe:', 'properly escaped');
99+
}
100+
});
101+
72102
QUnit.module("ember-htmlbars: {{#unbound boundHelper arg1 arg2... argN}} form: render unbound helper invocations", {
73103
setup: function() {
74104
Ember.lookup = lookup = { Ember: Ember };

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/* jshint scripturl:true */
2+
3+
var parsingNode;
4+
var badProtocols = {
5+
'javascript:': true,
6+
'vbscript:': true
7+
};
8+
9+
export default function sanitizeAttributeValue(element, attribute, value) {
10+
var tagName;
11+
12+
if (!parsingNode) {
13+
parsingNode = document.createElement('a');
14+
}
15+
16+
if (!element) {
17+
tagName = null;
18+
} else {
19+
tagName = element.tagName;
20+
}
21+
22+
if (value && value.toHTML) {
23+
return value.toHTML();
24+
}
25+
26+
if ((tagName === null || tagName === 'A') && attribute === 'href') {
27+
parsingNode.href = value;
28+
29+
if (badProtocols[parsingNode.protocol] === true) {
30+
return 'unsafe:' + value;
31+
}
32+
}
33+
34+
return value;
35+
}

‎packages/ember-views/lib/views/view.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import {
5555
read,
5656
isStream
5757
} from "ember-metal/streams/utils";
58+
import sanitizeAttributeValue from "ember-views/system/sanitize_attribute_value";
5859

5960
function K() { return this; }
6061

@@ -2193,7 +2194,8 @@ View.views = {};
21932194
View.childViewsProperty = childViewsProperty;
21942195

21952196
// Used by Handlebars helpers, view element attributes
2196-
View.applyAttributeBindings = function(elem, name, value) {
2197+
View.applyAttributeBindings = function(elem, name, initialValue) {
2198+
var value = sanitizeAttributeValue(elem[0], name, initialValue);
21972199
var type = typeOf(value);
21982200

21992201
// if this changes, also change the logic in ember-handlebars/lib/helpers/binding.js

0 commit comments

Comments
 (0)