angular-toastr icon indicating copy to clipboard operation
angular-toastr copied to clipboard

allowHtml prone to XSS-Vulnerabilities

Open bedag-moo opened this issue 7 years ago • 5 comments

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)

bedag-moo avatar Feb 19 '18 17:02 bedag-moo

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?

Foxandxss avatar Feb 19 '18 17:02 Foxandxss

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.

bedag-moo avatar Feb 19 '18 17:02 bedag-moo

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.

Foxandxss avatar Feb 19 '18 19:02 Foxandxss

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 avatar Feb 20 '18 15:02 bedag-moo

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 ...

bedag-moo avatar Apr 16 '18 20:04 bedag-moo