globaldev / angular-emoji-filter

An AngularJS filter for replacing emoji codes with emoticons
http://dev.venntro.com/angular-emoji-filter/
Other
125 stars 35 forks source link

Security implications of ng-bind-html-unsafe #7

Open henrik opened 10 years ago

henrik commented 10 years ago

I'm very new to Angular, but I believe specifying the usage as ng-bind-html-unsafe="message | emoji" comes with a big security risk – if that message comes from a user of the service, they could add <script>alert(123)</script> to their message to annoy others, or more dangerously, add JS to steal cookies/sessions, reroute forms etc.

I assume it must be possible to implement something for Angular that trusts the emoji HTML without trusting any other HTML. Perhaps that precludes using a filter? If so, maybe that's the way to go.

At the very least, and before doing anything else, maybe it would be a good idea to mention these security implications in the README?

weyert commented 10 years ago

Well, the backend should avoid XSS so it's fully into your control whether you want to support

henrik commented 10 years ago

@weyert, I don't see how it would be the back-end's job to protect you from XSS – that's typically handled by proper escaping on the front-end. There's no guarantee there even is a back-end with an Angular app.

But even if we assume it is the back-end's responsibility (which I disagree with), the README should then mention that since the front-end makes no attempts to protect you, you must consider this on that end.

weyert commented 10 years ago

Well, the back-end should also check for XSS and hence escape the content. That's how I always did it. Never trust front-end content.

henrik commented 10 years ago

Well, never trust user-provided content, certainly. That doesn't mean it has to be escaped on the back-end specifically – as long as you can't cause mischief for someone else by hacking your own client.

This library could most likely be changed to avoid the current XSS risk without involving other parts like a possible backend (what if there is none?). In my view that would be much preferable.

Such a fix might simply entail sanitizing the HTML string after inserting images into it, using Angular's $sanitize.

As long as the evildoer can't change the client-side code for other clients (e.g. by XSS…) a client-side fix like this is fine.

weyert commented 10 years ago

Sounds fair :-)

Sent from my iPhone

On 2 Aug 2014, at 15:44, Henrik Nyh notifications@github.com wrote:

Well, never trust user-provided content, certainly. That doesn't mean it has to be escaped on the back-end specifically – as long as you can't cause mischief for someone else by hacking your own client.

This library could most likely be changed to avoid the current XSS risk without involving other parts like a possible backend (what if there is none?). In my view that would be much preferable.

Such a fix might simply entail sanitizing the HTML string after inserting images into it, using Angular's $sanitize.

As long as the evildoer can't change the client-side code for other clients (e.g. by XSS…) a client-side fix like this is fine.

— Reply to this email directly or view it on GitHub.