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

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

Open
henrik opened this issue Jun 13, 2014 · 5 comments
Open

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

henrik opened this issue Jun 13, 2014 · 5 comments

Comments

@henrik
Copy link

henrik commented Jun 13, 2014

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
Copy link

weyert commented Aug 2, 2014

Well, the backend should avoid XSS so it's fully into your control whether you want to support <script>alert(123)</script>

@henrik
Copy link
Author

henrik commented Aug 2, 2014

@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
Copy link

weyert commented Aug 2, 2014

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
Copy link
Author

henrik commented Aug 2, 2014

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
Copy link

weyert commented Aug 2, 2014

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.

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