Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat($interpolate): escaped interpolation expressions #7517

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented May 20, 2014

This CL enables interpolation expressions to be escaped, by prefixing each
character of their start/end markers with a REVERSE SOLIDUS U+005C, and to
render the escaped expression as a regular interpolation expression.

Example:

<span ng-init="foo='Hello'">{{foo}}, \\{\\{World!\\}\\}</span> would be
rendered as:
<span ng-init="foo='Hello'">Hello, {{World!}}</span>

This will also work with custom interpolation markers, for example:

 module.
  config(function($interpolateProvider) {
    $interpolateProvider.startSymbol('\\\\');
    $interpolateProvider.endSymbol('//');
  }).
  run(function($interpolate) {
    // Will alert with "hello\\bar//":
    alert($interpolate('\\\\foo//\\\\\\\\bar\\/\\/')({foo: "hello", bar: "world"}));
  });

This change effectively only changes the rendering of these escaped markers,
because they are not context-aware, and are incapable of preventing nested
expressions within those escaped markers from being evaluated.

Therefore, backends are encouraged to ensure that when escaping expressions
for security reasons, every single instance of a start or end marker have each
of its characters prefixed with a backslash (REVERSE SOLIDUS, U+005C)

Closes #5601

@caitp
Copy link
Contributor Author

caitp commented May 20, 2014

@IgorMinar this is the simplest possible approach to this, but it has caveats which I'm not a fan of, particularly the being unaware of context. It's your call, since it minimizes the code size changes, but I'm not a fan of it.

}));


it('should evaluate expressions between escaped start/end symbols', inject(function($interpolate) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I really don't like about this version. (There are other things too, but this is the most problematic). If people are ok with this (and certainly many will be), then I'm all for it. But I see this as bad.

In my view, escaped expression markers should be fully protected from having their contents evaluated in any fashion

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment for this test that states that the server is responsible for ensuring that all bindings are properly escaped, if that doesn't happen, we won't go out of our way to ignore bindings in improperly escaped string.

@IgorMinar
Copy link
Contributor

This looks good to me. Can you please add docs for the $interpolate server that will describe how this works and what the requirements for the server are. Especially:

  • server must escape all occurrences of {{ and }} in all possible encodings (unicode, html entity, what not)
  • server must escape all occurrences of < and > in all possible encodings. no tags must make it into the template unencoded.

This CL enables interpolation expressions to be escaped, by prefixing each character of their
start/end markers with a REVERSE SOLIDUS U+005C, and to render the escaped expression as a
regular interpolation expression.

Example:

`<span ng-init="foo='Hello'">{{foo}}, \\{\\{World!\\}\\}</span>` would be rendered as:
`<span ng-init="foo='Hello'">Hello, {{World!}}</span>`

This will also work with custom interpolation markers, for example:

     module.
       config(function($interpolateProvider) {
         $interpolateProvider.startSymbol('\\\\');
         $interpolateProvider.endSymbol('//');
       }).
       run(function($interpolate) {
         // Will alert with "hello\\bar//":
         alert($interpolate('\\\\foo//\\\\\\\\bar\\/\\/')({foo: "hello", bar: "world"}));
       });

This change effectively only changes the rendering of these escaped markers, because they are
not context-aware, and are incapable of preventing nested expressions within those escaped
markers from being evaluated.

Therefore, backends are encouraged to ensure that when escaping expressions for security
reasons, every single instance of a start or end marker have each of its characters prefixed
with a backslash (REVERSE SOLIDUS, U+005C)

Closes angular#5601
@caitp caitp closed this in e3f78c1 May 20, 2014
RichardLitt pushed a commit to RichardLitt/angular.js that referenced this pull request May 24, 2014
This CL enables interpolation expressions to be escaped, by prefixing each character of their
start/end markers with a REVERSE SOLIDUS U+005C, and to render the escaped expression as a
regular interpolation expression.

Example:

`<span ng-init="foo='Hello'">{{foo}}, \\{\\{World!\\}\\}</span>` would be rendered as:
`<span ng-init="foo='Hello'">Hello, {{World!}}</span>`

This will also work with custom interpolation markers, for example:

     module.
       config(function($interpolateProvider) {
         $interpolateProvider.startSymbol('\\\\');
         $interpolateProvider.endSymbol('//');
       }).
       run(function($interpolate) {
         // Will alert with "hello\\bar//":
         alert($interpolate('\\\\foo//\\\\\\\\bar\\/\\/')({foo: "hello", bar: "world"}));
       });

This change effectively only changes the rendering of these escaped markers, because they are
not context-aware, and are incapable of preventing nested expressions within those escaped
markers from being evaluated.

Therefore, backends are encouraged to ensure that when escaping expressions for security
reasons, every single instance of a start or end marker have each of its characters prefixed
with a backslash (REVERSE SOLIDUS, U+005C)

Closes angular#5601
Closes angular#7517
@idupree
Copy link

idupree commented Jun 25, 2014

While this change thankfully makes it possible to write text that expands to {{, it makes it impossible to write text that expands to \{\{, as best I can tell. You could require all backslashes in Angular-processed text to be escaped (i.e. Angular converting \\ to \), and then it would be possible, albeit ugly, to write any literal string.

Alternately, the suggested workarounds in #5601 (comment) could be documented as the official way to escape Angular's curly braces. They appear to me to be a sufficient escaping system that is no more fragile than this commit's way of backslash-escaping Angular's interpolation markers. It's true that {{'{{'}} riskily adds single-quotes, and {{ DOUBLE_OPEN_BRACE }} requires DOUBLE_OPEN_BRACE to be defined. So you could add, say, NG_DOUBLE_OPEN_BRACE to the default scope environment to make it a solution anyone can easily use. Or make something else within curly brackets that's not syntactical JavaScript magically expand to {{; say, {{\}}.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XSS issues with server-rendered Angular templates
3 participants