Disallow interpolation on ng-app element to allow safe mixing of client and server-side templating

[timbertson]

• Do you want to request a feature or report a bug?

Feature

• What is the current behavior?

Interpolation {{ ... }} is automatically handled in any child of the main ng-app element.

• What is the expected behavior?

A new mode or flag should be added which prevents {{ ... }} from being interpolated in the main document template (i.e. the ng-app element). {{ ... }} will still be supported in templates.

• What is the motivation / use case for changing the behavior?

Mixing client-side and server-side templating has long been discouraged in angular, however this is not always easy advice to follow. When adding angular to an existing (say) rails app, it's very inconvenient to suddenly need to replace all server-side templating with client-side templating, even when there's no actual need for the logic to be client-side. And if you don't have a strong separation between server & client side templates (e.g. maybe you reuse a partial in multiple pages), it's all too easy for server-side templating to sneak into the client-side templates and cause security issues. Without dropping server-side rendering throughout your entire app, it's very difficult to be sure that you're using angular safely.

While I agree that mixing should be discouraged, angular is pretty close to allowing some safe mixing. All of the directive-related syntax is a superset of XML, so typical server-side content escaping prevents directive-injection. It's only interpolation-injection which the server cannot protect against, because it doesn't know which part of the page needs to be angular-aware.

If you could disallow {{...}} from being interpreted in the initial document template (i.e the ng-app element) but still allow it in all other templates, it's much easier for applications to ensure that no server-side templating can sneak in (e.g. serving plain .html templates). This would not cause any loss of power - any instances of {{...}} directly in the document body will work correctly once extracted into a template or directive.

• Which version of Angular, and which browser and OS does this issue affect? Did this work in previous
  versions of Angular? Please also test with the latest stable and snapshot versions.

N/A - feature request

[lgalfaso]

$compiler is not aware of what is part of the initial compilation and what is part of a directive, and should not be aware of this. I think that implementing this mode is a bad idea as it enables users to do dynamic server-generated templates that is very very dangerous and should be discouraged. If there are parts of a template that generate content server side, then the developer can create a directive that would prevent any directive to be rendered inside that element (this includes interpolation) using terminal: true.

I will keep this open so people can comment.

[petebacondarwin]

Also there are plenty of other places in HTML where you can provide an Angular expression, such as ng-click, ng-if and so on, which could potentially allow an attack vector. So blocking {{ ... }} would be insufficient.

[petebacondarwin]

@timbertson thanks for making this suggestion. We are always pleased to consider improving the security of the framework. I don't think this is valid for actually making things safer without encouraging people to think they are safe when they are not.

This idea did make me think though, that one could personally limit your own Angular apps so that the bit of the page that was running the app always looked like this:

<my-app ng-app="my-app"></my-app>

And then the entire app would be a single element component, which would not be allowed to expose any binding of any kind (interpolation or otherwise) on the main HTML page. This component would be the root of the Angular app and have its own template that could contain Angular expressions.

Outside of this element you could conceivably allow server side rendering. But realise that even that is not secure.. What if someone manages to inject <div ng-app="my-app">{{ some evil expression}}</div> higher up your page before the "official" ng-app?

If you have any more ideas then please do send them in.

[timbertson]

I understand that this could be a lot of wok or might be architecturally awkward. But I'm not convinced of the other arguments.

Your rejections seem mainly based on "it's still not safe". But perhaps you misunderstand. I'm not trying to make it so that "angular and server side scripting is totally safe". I'm trying to make it so that "a diligent developer can mix these two technologies without opening up security holes". The way angular is set up right now, it's basically impossible to do so because:

1. The way angular requires ng-app on the root element means that it's very difficult to apply angular to only the parts of the page that need it (e.g. if you have an angular navbar and an angular form, you need ng-app on their common parent, which could easily be <body>).

2. You can opt out of angular with ng-non-bindable, but that's a blacklist. If I could use ng-enable-binding (with the default of non-binding) that would be another solution (I think it would work quite well)

I believe that with my proposed modifications, using angular and server-side rendering on a page would be no harder than it already is to keep HTML safe (which we are getting quite good at now).

That is, of course you'd still have issues if you allowed users to have control over "privileged" HTML attributes like ng-if, ng-click, etc. But those risks already exist in HTML (onclick, etc). Just because angular adds additional attributes that should not be user-controlled, does not mean the effort is a waste. I am quite capable of keeping user input out of onclick attributes, and I would be just as capable keeping it out of ng-* attributes in my server-side templating.

Similarly, you argue that the user could inject <div ng-app="..."</div> earlier in the page. If you allow the user to inject arbitrary HTML, the user could also inject a <script>. It increases the attack area compared to native HTML, but the same mechanisms (whitelisting & escaping) will still work even if the server is completely unaware of angular.

The suggestion for terminal: true would be good, except that I specifically want to allow directives, but not interpolation. The important difference is that directives cannot be injected by user data (since they are XML syntax, and I do no allow arbitrary XML injection). Crafting safe HTML is very well understood at this point, so it's entirely possible to mix user input with HTML_angular directives safely. Interpolation is the only non-XML syntax that needs fixing here. Any chance of reconsidering this request?

[lgalfaso]

If you create a directive that is terminal and has a low enough priority, then all other directives will be executed and the text interpolated inside will not. Eg.

<div ng-app="...">
  [...]
  <div some-directive>
    <span another-directive do-not-interpolate>#{user.name}</span>
  </div>
  [...]
</div>

then someDirective will be executed, so will anotherDirective but doNotInterpolate will preven anything from #{user.name} to be executed.

[petebacondarwin]

Hi @timbertson - I understand that you suggestion would make it a bit harder to write unsafe templates for diligent developers. The trouble is that this would encourage non-diligent developers to go ahead and do this thinking it is safe when it is not.

The safest message we can deliver is not to mix scripting technologies at all.

The fact is that diligent developers can already use server side injection of user provided content, if they know what they are doing and suitably sanitize the content that is provided as a template that Angular will consume. But this is really tricky and no less so if you simply block interpolation.

I think that the suggestion from @lgalfaso is reasonable, yet still fraught with danger: perhaps user.name uses some trick to render as </span>{{ some evil expression }}. I would not like to be the person responsible for ensuring that this was impossible.

[lgalfaso]

if #{user.name} can render anything, then it can render </span><script>alert()</script> and you have other things to worry about.

[petebacondarwin]

Yes of course. But I am suggesting that there are plenty of things that could slip through a basic server side sanitisation pass. If we could sanitize effectively for Angular then we wouldn't need to block interpolation either.

[timbertson]

The fact is that diligent developers can already use server side injection of user provided content, if they know what they are doing and suitably sanitize the content that is provided as a template that Angular will consume. But this is really tricky and no less so if you simply block interpolation.

That's not true though. We are very good at escaping HTML. Most frameworks these days do it for you automatically. e.g. in rails you don't need to do #{username | escape_for_html}, you just insert #{username} directly. The safe_html mechanism will ensure that html-sensitive characters (<>&,etc) are escaped automatically, because the username value is not tagged as a HTML fragment.

So it's quite easy (if you follow best practices) to produce safe HTML including angular directives using a modern framework like rails, because directives use HTML syntax. On the other hand, when interpolation is thrown into the mix, rails' safe html mechanism is useless because it doesn't know about the brace syntax. To maintain safety in the presence of interpolation, I would either have to (a) teach safe_html how to escape interpolation (I'm not sure if there are even hooks for this), or (b) manually pass all user text through an additional angular_escape function before inserting into a template.

@lgalfaso your example is a good illustration of the difference. If it were possible to inject a <script> tag, then it's game over with or without angular. My entire point is that it's relatively easy to protect against arbitrary HTML injection. But currently, it's almost impossible to prevent against interpolation injection. And crucially, interpolation is the only thing that existing HTML injection mechanisms do not protect us from - without interpolation, angular would be just as safe as HTML.

[petebacondarwin]

I don't agree that html is always escaped safely by server side sanitizers. There are plenty of sanitizers out there that whitelist lots of "safe" HTML elements. Sure they strip out onclick attributes and so on, but you would need to get them to strip out all ng-... attributes and also any custom attribute directives that are in your app too.

[petebacondarwin]

I guess what I am trying to say is that I agree with you that blocking interpolation would make it easier for you to write safer server side templates, but that it would encourage not so diligent developers to do things that could easily shoot themselves in the foot.

[timbertson]

@petebacondarwin I think you are talking about something different. Yes, there are programs which accept arbitrary user-input HTML and try to "make it safe". Those are risky, and would need additional awareness to make them safe for use with angular templates. But that's not what I'm talking about. When I talk about server-side templating, I mean templating which always escapes user input - you do not take HTML and "make it safe" by whitelisting certain tags, you construct safe HTML by taking template and inserting user-suplpied values after html-escaping them. This prevents all user-directed insertion of HTML tags or attributes - there is nothing to "slip through" because all characters which could be meaningful to a HTML parser are always escaped.

[gkalpak]

If I understand correctly, escaping the interpolation markers ({{/}} or whatever they are) would solve this issue. Isn't it quite straight-forward to escape them as part of the server-side sanitization (although the framework won't do it automatically, you have to explicitly do this) ?

(See escaped interpolation on the docs.)

If you don't want to do this on the server side, wouldn't this problem be solved with a directive that executes with a very high priority and escapes all interpolations in it's sub-tree ?

[timbertson]

Isn't it quite straight-forward to escape them as part of the server-side sanitization (although the framework won't do it automatically, you have to explicitly do this) ?

No, because (e.g.) in rails the HTML escaping happens automatically. I don't believe there is any way of extending this to cover {{}} as well. So you go from "everything is escaped by default" to "you must remember to manually escape every piece of data", which is a huge step up in the amount of diligence required.

a directive that executes with a very high priority and escapes all interpolations in it's sub-tree ?

If possible, that sounds like it ought to work. I'll look into it.

[gkalpak]

If I understand the usecae corrctly, you want to have a template (e.g. index.html) be created by the server, so that:

• It may contain HTML markup (including directives, expressions etc) that you have specified (not the user).
• It may contain user input, sanitized by the server-side framework, so that it does not contain any HTML markup, only text (which might be the result of escaped HTML, but that's irrelevant). It may also contain interpolation (but we want to avoid evaluating that).
• The template should be compiled by Angular as usual (i.e. your directives should work as usual), but no interpolation should happen on text content (neither text content specified by you nor text content that is a result of escaped user input).

Is that correct ?

[timbertson]

@gkalpak that's right! Note that there's a difference between the document template (i.e. the ng-app element) and additional templates loaded either from the server or the templateCache. I want interpolation to be disabled on the document template as you describe, but to still be allowed in these other templates, since they are intended for angular's use, and it's easy to keep server-side templating out of them.

[gkalpak]

@timbertson, it's not as straight-forward as a config flag (but much more powerful): demo

The demo overloads the ngApp directive in order to provide the required behavior (as described in #14186 (comment)).
It is not specific to ngApp though. You can name the directive whatever you want and use it anywhere (although it's definitely a really bad idea to have server-side generated directive templates 😛).

[timbertson]

@gkalpak thanks, that's actually pretty close to what I'd want. I actually had a try at this myself, but was thrown by the fact that escaped interpolation markers are only unescaped if there's interpolation elsewhere in the same string (surely that's a bug? - #14196 ).

It still feels a little risky - if you hit a runtime error in this directive, or fail to load it, your page will quietly remain vulnerable. But I think it shows the promise quite well. Is there any chance something like this could be rolled into angularJS, so that folks who need it don't have to write it themselves?