Optionaly filter arguments of XSS-vulnerability-prone jqlite methods through $sce

[xtofian]

The JQuery API exposes several methods whose use in application code carries a high risk of introduction of XSS vulnerabilities.

For example, code such as myElement.html(val) results in XSS if val is (wholly or partially) derived from untrustworthy input, and not constructed in a way that ensures that sub-expressions have been appropriately sanitized and/or escaped for the context in which they appear in the HTML markup contained in val. I.e., the use of html(val) carries a similar risk of XSS vulnerabilities as would be present due to the use of ng-bind-html, if the latter did not address that risk by subjecting its argument expression to the $sanitize and $sce service.

Hence it would be desirable to subject arguments to XSS-prone jqlite APIs (such as .after(val), .before(html), .html(val), etc) to $sce as well.

Since this is a change that significantly changes behavior, it would need to be guarded by a configuration option.

Potential issues:

• How to inject $sce into jqlite?
• Some methods require sanitization that is data depedent. For example, el.attr('title', val) doesn't require any special sanitization or escaping on val. However, in el.attr('href', url), url needs to be sanitized as a safe URL.

[lgalfaso]

I am trying to understand what this issue is trying to address. If someone is able to run JavaScript in the context of an app, then any form of security measure is already gone. When would it be possible for someone to use after, before or html without already having full control?

[xtofian]

The purpose of this feature is not to mitigate attacks (i.e. prevent an attacker from doing things once they already have script execution in the origin). Rather, it's to prevent developers from accidentally introducing vulnerabilities into the application in the first place.

Some examples of bugs/vulnerabilities that can happen:

• Developer writes, el.html(text), where text is externally (i.e., potentially attacker) controlled. text usually doesn't contain markup, and the developer actually meant to write .text(text).
• Developer writes el.html('<b>' + important + '</b>') -- here they forgot to HTML-escape important.
• Developer writes el.html('<a href="' + url + '">...</a>') -- here they forgot to sanitize, and HTML-escape the URL.

In all of these cases, the app works perfectly fine in dev, test, and normal, non-malicious usage; the strings in question usually don't contain any troublesome characters.

IME it is very difficult for developers to always remember to apply correct sanitization and escaping when writing code that performs ad-hoc construction of HTML markup.

Hence the desire to create APIs that are always safe (by default sanitize or escape their parameters, unless they are supplied in the form of types that promise that the values are already safe for a given context).

For more background on this approach, see http://research.google.com/pubs/archive/42934.pdf.

[lgalfaso]

The main problem with changing the behavior of html, after and before is that jqLite (and jQuery) are not services, while $sce is a service that can be configured within the app. This is, elements wrapped generated when calling angular.element can be part of an app or not (and currently it is hard to know if if it is part of an app, of what app) so it would be hard to know what $sce should be used to sanitize the html.

[xtofian]

Would it be possible to provide a way to globally set the $sce to be used?
This certainly would be counter to proper DI idioms, but maybe it's OK --
this is something we'd want to take effect for the whole page (or even
origin).

On Sun, Jun 28, 2015 at 7:50 PM, Lucas Mirelmann notifications@github.com
wrote:

The main problem with changing the behavior of html, after and before is
that jqLite (and jQuery) are not services, while $sce is a service that
can be configured within the app. This is, elements wrapped generated when
calling angular.element can be part of an app or not (and currently it is
hard to know if if it is part of an app, of what app) so it would be hard
to know what $sce should be used to sanitize the html.

—
Reply to this email directly or view it on GitHub
#12222 (comment)
.

[mgol]

Making jqLite use $sce under the hood would create a divergence from jQuery. Unless the proposal was to also patch jQuery methods but I'd strongly advise against that; we used to patch jQuery methods before AngularJS 1.3.0 and it caused a lot of grief.

jQuery 1.12/2.2 introduced jQuery.htmlPrefilter through which the internal buildFragment passes the input. It's exposed so that it can be set to another value if someone wants stricter control over security of those methods.

Would adding something like that to jqLite satisfy your needs, @xtofian?

[xtofian]

Thanks @mgol I didn't know about htmlPrefilter. That does indeed sound like it could address the concern.

@rjamet what do you think?

[rjamet]

I'm not sure that would help a lot: the comments from last year are right in that jqLite lives outside of the Angular app. With something like this, we could hook the usual sanitizer / safe types combination, but since it has no reference to the Angular app, meshing its policies with the app's $sce would be brittle unless we duplicate infrastructure, which wouldn't be great either. Also, this htmlprefilter doesn't seem to provide any context, and it also doesn't hook attributes. We could certainly do some checks with it, but probably not in a really useable and complete way, so I'm not confident it's worth the effort.