Skip to content

Commit

Permalink
Also prevent binding against text content inside <script> tags
Browse files Browse the repository at this point in the history
  • Loading branch information
SteveSanderson committed Jul 1, 2013
1 parent 0f6e3c9 commit c61abc7
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
36 changes: 36 additions & 0 deletions spec/bindingAttributeBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,4 +406,40 @@ describe('Binding attribute syntax', function() {
ko.applyBindings({}, testNode);
// Should not throw any errors
});

it('Should not bind against text content inside <script> tags', function() {
// Developers won't expect or want binding to mutate the contents of <script> tags.
// Historically this wasn't a problem because the default binding provider only acts
// on elements, but now custom providers can act on text contents of elements, it's
// important to ensure we don't break <script> elements by mutating their contents.

// First replace the binding provider with one that's hardcoded to replace all text
// content with a special message, via a binding handler that operates on text nodes
var originalBindingProvider = ko.bindingProvider.instance;
ko.bindingProvider.instance = {
nodeHasBindings: function(node, bindingContext) {
return true;
},
getBindingAccessors: function(node, bindingContext) {
if (node.nodeType === 3) {
return {
replaceTextNodeContent: function() { return "replaced"; }
};
} else {
return originalBindingProvider.getBindingAccessors(node, bindingContext);
}
}
};
ko.bindingHandlers.replaceTextNodeContent = {
update: function(textNode, valueAccessor) { textNode.data = valueAccessor(); }
};

// Now check that the only text nodes whose contents are mutated are the ones
// *not* inside <script> tags.
testNode.innerHTML = "<p>Hello</p><script>alert(123);</script><p>Goodbye</p>";
ko.applyBindings({ sometext: 'hello' }, testNode);
expect(testNode).toContainHtml('<p>replaced</p><script>alert(123);</script><p>replaced</p>');

ko.bindingProvider.instance = originalBindingProvider;
});
});
13 changes: 11 additions & 2 deletions src/binding/bindingAttributeSyntax.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
(function () {
(function () {
ko.bindingHandlers = {};

// The following element types will not be recursed into during binding. In the future, we
// may consider adding <template> to this list, because such elements' contents are always
// intended to be bound in a different context from where they appear in the document.
var bindingDoesNotRecurseIntoElementTypes = {
// Don't want bindings that operate on text nodes to mutate <script> contents,
// because it's unexpected and a potential XSS issue
'script': true
};

// Use an overridable method for retrieving binding handlers so that a plugins may support dynamically created handlers
ko['getBindingHandler'] = function(bindingKey) {
return ko.bindingHandlers[bindingKey];
Expand Down Expand Up @@ -190,7 +199,7 @@
if (shouldApplyBindings)
shouldBindDescendants = applyBindingsToNodeInternal(nodeVerified, null, bindingContext, bindingContextMayDifferFromDomParentElement)['shouldBindDescendants'];

if (shouldBindDescendants) {
if (shouldBindDescendants && !bindingDoesNotRecurseIntoElementTypes[ko.utils.tagNameLower(nodeVerified)]) {
// We're recursing automatically into (real or virtual) child nodes without changing binding contexts. So,
// * For children of a *real* element, the binding context is certainly the same as on their DOM .parentNode,
// hence bindingContextsMayDifferFromDomParentElement is false
Expand Down

0 comments on commit c61abc7

Please sign in to comment.