XSS issues with server-rendered Angular templates

[triskweline]

Dear Angular community,

we would greatly appreciate any comments on the issue below.

Overview of the issue:

When rendering Angular templates with a server-side templating engine like ERB or Haml it is easy to introduce XSS vulnerabilities. These vulnerabilities are enabled by AngularJS evaluating user-provided strings containing interpolation symbols (default symbols are {{ and }}).

The standard mitigation strategy by templating engines is to always escape meaningful symbols in strings (unless they are explicitely marked as "safe"). Unfortunately the inability to escape AngularJS interpolation symbols makes it hard to apply this strategy here.

Reproducing the issue:

We are using AngularJS for the frontend and Rails in the backend. While we have written full Single Page Applications with AngularJS, we are often embedding small snippets of AngularJS in server-rendered views. We love that AngularJS lets us replace jQuery spaghetti with simple code like this:

<div ng-show="detailsShown">
  <%= user.profile %>
</div>

<span ng-click="detailsShown = true">Show details</span>

The problem is that the code snippet above enables Cross Site Scripting (XSS). If the server-parsed expression user.profile contains AngularJS markup like "Foo {{signOut()}} Bar", it will trigger functions on the current scope, allowing an attacker to trigger side effects on another computer.

Proposed fix:

The standard mitigation strategy by templating engines is to always escape symbols that have meaning for downsteam parsers, unless they are explicitely marked as "safe". E. g. if the server-parsed expression user.profile in the code example above would yield the string "Foo <script>alert('owned')</script> Bar", Rails, Haml, etc. would automatically replace the angle brackets with HTML entities.

To allow HTML tags to be rendered without escaping, the developer needs to explicitely mark a string as "safe" like so:

<%= user.profile.html_safe %>

(AngularJS actually uses this approach itself, when inserting the results of an evaluated Angular expression into the DOM.)

A good default for people rendering Angular templates using server-side templating engines would be to make templating engines automatically escape Angular interpolation symbols unless an evaluated string is explictely flagged as "safe". A patch for engines like ERB or Haml would be easily created, but unfortunately Angular's $interpolate service does not recognize any kind of escape symbol.

We propose that the $interpolate service should recognize escaped interpolation symbols like this:

Foo \{\{signOut()\}\} Bar

... or maybe a second pair of start/end symbols.

When encountering escaped interpolation symbols, the service would not evaluate the enclosed expression. Instead it would strip the escape symbols from the string.

(Note that we first tried to replace the curly braces with their HTML entities, which would have the added bonus of being output-neutral outside of an ng-app element. However, entitized braces are still recognized and their contents evaluated by Angular. We're not sure if the entities are expanded by Angular or by the browser.)

Plea to not dismiss this issue

We understand that this issue is not an actual bug in AngularJS, but (like all XSS issues) is caused by combining multiple technologies that are unaware of each other. Still we believe that AngularJS' ability to co-exist with non-Angular code in the same DOM is one of AngularJS greatest strengths. It sets Angular apart from frameworks like Ember, which need to own the entire page.

By offering a way to escape interpolation symbols (or maybe some other solution we didn't think of), AngularJS could be used safely in combination with upstream templating engines.

What do you think?

[caitp]

So, there's a problem with this, and I'll tell you what it is, with an example:

http://jsfiddle.net/KKDAE/

When it comes to "actually" escaping things into HTML entity references, these are going into text nodes, and by the time the compiler looks at them, they are already going to be "dangerous".

What you can do is escape these into something meaningless which the browser won't turn into an expression which will be interpolated, thus preventing execution.

I'm sorry if it sounds like I'm dismissing the issue, but it sounds to me like you have the tools at your disposal to prevent unwanted expressions from getting rendered already (at the cost of putting unexpected/hard to read stuff in the view)

As you suggest, \{\{ wouldn't be interpolated (by default, unless you tell the interpolate service to deal with this), so that's an option for you --- it just makes stuff look a bit gross. http://jsfiddle.net/q8B4p/1/

[tdierks]

I definitely agree. Software authors need to be able to control such metadata; given the stream-expressed encoding of HTML, this requires the ability to escape metadata instructions to reduce the privilege level of potentially-harmful content.

[caitp]

Right, but you can really already do exactly that --- if you simply escape, or even totally remove interpolation expressions or untrusted attributes from your rendered templates, you avoid this issue entirely. I'm not sure what we can really do in angular-land to deal with unescaped templates, we have no idea that you aren't serving exactly what you intended to.

[triskweline]

Thanks for your comments and especially @caitp for for taking the time to write up the fiddles.

I agree that we can fix the security aspect of this by simply replacing the interpolation markers with something else. But then we have changed the contents of the view, which can produce other issues.

I understand that using HTML entities cannot be used since the browser will expand them before Angular starts interpolating. My idea was that since Angular can't know if an interpolation mark is safe to parse or not, there could be a way to tell Angular if an {{ is meant to be a verbatim {{ that should be rendered so.

One way to tell Angular to render a verbatim {{ could be to use a second pair of interpolation marks, which Angular would print as {{ instead of evaluate the contents.

[caitp]

The issue with that is that, if there was another interpolation expression in the text node, then this would work for only one digest, because the following digest it would no longer be escaped (if you want to render it as an unescaped string). Having a way to flag an expression as being ignored might be nice, but currently it doesn't really fit into the way things work, and would likely be a pretty big/complicated change.

For now, the best bet is to simply escape your stuff server-side, there really isn't much else to do about it :(

[tdierks]

I don't think it's possible to escape this server-side without changing the content. If a profile (in the example) happens to have "{{" in it, you can't serve that content safely without changing how it looks to the end-user (e.g. inserting a space into the middle). Since {{ could occur in other contexts that are harder to change (e.g. it could be part of an image URL), it's even more complex.

I don't know how to fix this, but I believe it's a real and significant issue.

[caitp]

Yeah, as said, we'll discuss this to see if we can make a change to the interpolate service / html compiler to resolve this.

But the issue is always going to come back --- once you escape {{ with {-{, what happens when you need {-{ in a template? Where does it end? At a certain point you need to constrain the content

[tdierks]

Sure, this is a solved problem for all escaping systems. You could make people send {{{ when they want to render {{; you'd just trim one { off of any string of {'s longer than 2. I agree that it's not obvious how to do this without adding a metadata tagging system to the in-memory copy of the page, but I don't know enough about the internals to make a specific implementation suggestion.

[tbosch]

Thanks for reporting this.
How about using a character that looks like curly braces but is not? See http://stackoverflow.com/questions/13535172/list-of-all-unicodes-open-close-brackets

Normal braces: {
Substitues: ｛ : 0xFF5B ❴ : 0x2774

[caitp]

That's not a bad solution for them, and is probably significantly cheaper than re-arching the $interpolate service to keep track of escaped expressions or whatever.

[triskweline]

Is it true that {{ marks (once unescaped by an imaginary $interpolate service) would be evaluated as Angular expression by a following digest? How could I verify this with code? Because if that is the case the broader security implications of this scare me. Doesn't this me that any client-side integration that inserts unescaped text nodes into the DOM can make Angular run Javascript?

Some examples:

• We embed Google Maps with angular-ui and an attacker gets me to open a Maps popup where some business listing contains double curly braces => Javascript is run
• We embed a Javascript-provided RSS feed summary of some blog. The owner of that blog adds a headline with double curly braces => Javascript is run
• We embed some Javascript search widget that echoes the value of a ?query=search%20string query parameter. An attacker gets me to open a URL with a query param of their choice => Javascript is run

Unless we can make any DOM-changing Javascript library escape their curly braces, is there a way to remedy this? Would we need to recommend Angular users to never use libraries that insert uncontrolled text nodes into the DOM?

[triskweline]

@tbosch Thanks for the creative suggestion :) We tried something similar, but instead used ​, a zero-width space, which is a thing that exists.

We abandoned the idea because it broke inlined Javascript. We were also worried it would confuse users who copy text from the page and use it for something else (e.g. this very page).

We're currently escaping {{ to { { (spaces left, middle and right) which felt less magic and preserves most inlined Javascript. We're still unhappy that we're changing the content at all.

[caitp]

Is it true that {{ marks (once unescaped by an imaginary $interpolate service) would be evaluated as Angular expression by a following digest?

well, it's sort of like this (I believe, and it's possible I'm wrong, due to not having touched the $interpolate service much)

If you give us something which the browser's HTML parser decides looks like {{, then during the HTML compilation phase, it's going to become a watched interpolation expression (it's possible that it does need the matching }} though, it wouldn't be hard to check) --- so, once the interpolation expression is built, then the string literals don't really matter (I believe), the actual text content of the node is never going to matter again, unless $interpolate is called again... so the original interpolation listener is basically going to replace the text node content every digest, and evaluate the interpolated expression every digest.

There are a few parts of this where I could be mistaken, so it may theoretically be possible to make this work well, but I'm not totally sure it's the right thing to do, I think @tbosch's proposal would make a lot more sense.

[xrg]

On Thursday 02 January 2014, Henning Koch wrote:

<%= user.profile %>

What about the ng-non-bindable directive?

<%= user['bobby.tables'].profile %>

http://docs.angularjs.org/api/ng.directive:ngNonBindable

Disclaimer waiver: When you send me an unencrypted email, you implicitly
allow me, or any 3rd person reading our mails, to do anything I/they wish
with your data (including presenting them in public). Your disclaimer, thus,
is void. If you had wanted a private communication, you should have used
encryption in the first place.