Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

itemToString does not escape values by default #516

Closed
wesvetter opened this issue Jun 17, 2016 · 3 comments
Closed

itemToString does not escape values by default #516

wesvetter opened this issue Jun 17, 2016 · 3 comments

Comments

@wesvetter
Copy link
Contributor

It seems like there is a possible vulnerability in using the List editor with the Object item-type. Modal.prototype.itemToString does not escape the values by default which makes it vulnerable to XSS attacks.

Here is a simple example: https://jsfiddle.net/wesvetter/sh57z7ba/2/

If this is the intended behavior then a note should be added to the README so that users are aware of the behavior.

@glenpike
Copy link
Collaborator

glenpike commented Sep 5, 2016

Half inclined to ignore this because you're fiddle is written in CoffeeScript and I have to learn something else just to grok the problem... :p

There is a workaround - you can supply your own itemToString function as part of the schema if it's possible that you need to escape data.

I will also mention in the documents.

I've done a JavaScript JSFiddle of the workaround ;)

@wesvetter
Copy link
Contributor Author

If you're displaying any user-submitted data, I'd advocate overriding it at the top level with the safe (escaped) version, and passing an unescaped version at the schema-level should you need an unescaped itemToString.

In other words:

// main.js
window.Backbone.Form.editors.List.Modal.prototype.itemToString =
 function() {
   ... // escaped version
}

// foo.js
new Backbone.Form({
  schema: {
    itemToString: function() { ... } // unescaped version
  }
})

It's worth noting that if you want markup in your itemToString method you still need to remember to escape user-submitted values. e.g.:

itemToString: function(value) { return key + ": <strong>" + _.escape(value) + "</strong>" })

I'd also consider using _.escape rather than $('div').html(value).text().

@glenpike
Copy link
Collaborator

glenpike commented Sep 7, 2016

Okay, it probably makes sense to "play safe" by default. As this would be a breaking change, I will probably slate it for later - if we are to release non-breaking changes soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants