Skip to content

Persistent DOM XSS found #1540

Closed
b1shan opened this Issue Aug 10, 2012 · 11 comments

5 participants

@b1shan
b1shan commented Aug 10, 2012

Hello folks- example at http://backbonejs.org/examples/todos/index.html is vulnerable to DOM XSS.

Since it uses localStorage this can turn more grave by mimicking a persistent XSS by chaining Clickjacking, Drag & Drop or in a shared environment.

PoC exploit code: enter following in the input box and hit enter "" <img src=0 onerror=alert(0)>

Bish

@knowtheory
Collaborator

I can't say i'm surprised. It is just a little demo app. That said, a pull request sanitizing the contents pulled from the form prior to creating a new Todo I'm sure would be welcome.

@b1shan
b1shan commented Aug 10, 2012

And people get inspired from these demo apps ending up using in production code.

I suggest using Handlebars / Mustache as your view engine so you get the benefit of auto escaping.

The web is plagued with XSS and projects like Backbone can do good to community by adopting safer methods as defaults. We all know with HTML5 these things are going to be even more wicked.

@knowtheory
Collaborator

You seem to be under the impression that I have disagreed with you when I haven't. Like I said, pull requests welcome.

The purpose of the todo app is to demonstrate Backbone's basic feature set (which is also, i presume, why @addyosmani has adapted it to the myriad MVWTF frameworks out there), which is why I am unsurprised that as a simple toy app, it doesn't sanitize its input (yet).

That said, I don't think the main demo app should (or will) switch to handlebars/mustache, since again, the point is to show off the basic features of Backbone.

@addyosmani

I'm with @knowtheory on this one. As a toy/learning application, it's purpose is to just give developers the most basic view of what the framework is capable of.

Including Handlebars/Mustache here (if we're saying people are likely to copy to production, which is scary) might also be taken as Backbone suggesting users should be opting for a specific templating library over another, which you probably don't want to start arguments over.

That said, if a PR with auto-escaping is merged here, I'll happily consider for my TodoMVC apps.

@factormystic

There's value in doing it the a right way even in the examples, to help beginners get on a safe track. Can the templated properties simply be wrapped with _.escape? No additional dependencies needed.

@jashkenas
Owner

Even easier than that -- it's just a matter of using <%- value %> to interpolate.

@b1shan
b1shan commented Aug 11, 2012

as @factormystic agrees, my only request is to have examples that are "by design safe". Requiring devs to sanitize user input manually hasn't worked else XSS wouldn't still be the #1 web security issue even after more than a decade. I am no Backbone guru. But Mustache or equivalent does a fair job. If you have something even simpler, great.

I agree ith @addyosmani that Backbone might be promoting something in that manner, but then there is YUI Y.App http://yuilibrary.com/yui/docs/app/app-contributors.html that uses Handlebars. May be you could just put a notice that "you could use one of your choice" like YUI has done in main sections.

@knowtheory
Collaborator

The Backbone docs already note that you can use the templating library of your choice: http://backbonejs.org/#FAQ-tim-toady

@knowtheory
Collaborator

Fixed this with 71d0fe3

Disappointed that @b1shan is trolling the issue rather than having submitted a change, but oh well.

@knowtheory knowtheory closed this Aug 14, 2012
@b1shan
b1shan commented Aug 14, 2012

@knowtheory i did not understand your disappointment. Intention was definitely not that.

I am not saying you did not fix it.

I am sharing my concern of not promoting safe by default security practices amongst developers. This is a common industry issue that has been pointed by security folks earlier as well. In my other tweet I pointed why Chrome XSS Auditor DOM XSS protection won't protect most users. That doesn't meet Chrome is bad. It's just that people need to know what they are getting at the end of the day,

@b1shan
b1shan commented Aug 14, 2012

And please note, I am doing a responsible disclosure like I did with Chrome bug report. Only after you fixed it, I shared it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.