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

allowHtml prone to XSS-Vulnerabilities #235

Open
bedag-moo opened this issue Feb 19, 2018 · 5 comments
Open

allowHtml prone to XSS-Vulnerabilities #235

bedag-moo opened this issue Feb 19, 2018 · 5 comments

Comments

@bedag-moo
Copy link

bedag-moo commented Feb 19, 2018

By trusting all HTML, toastr bypasses the XSS protection provided by angular:

    if (options.allowHtml) {
      toast.scope.allowHtml = true;
      toast.scope.title = $sce.trustAsHtml(map.title);
      toast.scope.message = $sce.trustAsHtml(map.message);

I think it is not toastr's place to assert that arbitrary HTML is safe for direct inclusion in the DOM.

(this actually gave rise to an XSS vulnerability in one of our applications)

@Foxandxss
Copy link
Owner

Really? I remember looking at this stuff too much and that is why I never let real angular to be executed within toasts. Shoulndn't trustAsHtml discard any XSS?

@bedag-moo
Copy link
Author

No, it asserts that the passed string comes from a trusted source and doesn't need any sanitizing ...

Or as the docs put it:

To be secure by default, AngularJS makes sure bindings go through that sanitization, or any similar validation process, unless there's a good reason to trust the given value in this context. That trust is formalized with a function call. This means that as a developer, you can assume all untrusted bindings are safe. Then, to audit your code for binding security issues, you just need to ensure the values you mark as trusted indeed are safe - because they were received from your server, sanitized by your library, etc. You can organize your codebase to help with this - perhaps allowing only the files in a specific directory to do this. Ensuring that the internal API exposed by that code doesn't markup arbitrary values as safe then becomes a more manageable task.

In the case of AngularJS' SCE service, one uses $sce.trustAs (and shorthand methods such as $sce.trustAsHtml, etc.) to build the trusted versions of your values.

You can also check the implementation. Note that the parameter to trustAs is called trustedValue.

@Foxandxss
Copy link
Owner

I see. I always assumed that the toasts were generated by a trusted source and never by the user and I think that is the right idea.

@bedag-moo
Copy link
Author

Please document such assumptions.

that is the right idea

Well, we used it to display an error message received from the server ... which happened to quote invalid user input ...

@bedag-moo
Copy link
Author

Can't you just remove the $sce.trustAsHtml calls? This should cause angular to automatically sanitize the HTML, removing dangerous tags, but leaving harmless tags as they are ...

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