Skip to content
Permalink
Browse files

refactor($parse): remove Angular expression sandbox

The angular expression parser (`$parse`) attempts to sandbox expressions
to prevent unrestricted access to the global context.

While the sandbox was not on the frontline of the security defense,
developers kept relying upon it as a security feature even though it was
always possible to access arbitrary JavaScript code if a malicious user
could control the content of Angular templates in applications.

This commit removes this sandbox, which has the following benefits:

* it sends a clear message to developers that they should not rely on
the sandbox to prevent XSS attacks; that they must prevent control of
expression and templates instead.
* it allows performance and size improvements in the core Angular 1
library.
* it simplifies maintenance and provides opportunities to make the
parser more capable.

Please see the [Sandbox Removal Blog Post](http://angularjs.blogspot.com/2016/09/angular-16-expression-sandbox-removal.html)
for more detail on what you should do to ensure that your application is
secure.

Closes #15094
  • Loading branch information...
petebacondarwin committed Sep 6, 2016
1 parent 76d3daf commit 1547c751aa48efe7dbefef701c3df5983b04aa2e

This file was deleted.

Oops, something went wrong.

This file was deleted.

Oops, something went wrong.

This file was deleted.

Oops, something went wrong.

This file was deleted.

Oops, something went wrong.

This file was deleted.

Oops, something went wrong.

This file was deleted.

Oops, something went wrong.

This file was deleted.

Oops, something went wrong.
@@ -113,11 +113,11 @@ You can try evaluating different expressions here:
Angular does not use JavaScript's `eval()` to evaluate expressions. Instead Angular's
{@link ng.$parse $parse} service processes these expressions.

Angular expressions do not have access to global variables like `window`, `document` or `location`.
Angular expressions do not have direct access to global variables like `window`, `document` or `location`.
This restriction is intentional. It prevents accidental access to the global state – a common source of subtle bugs.

Instead use services like `$window` and `$location` in functions called from expressions. Such services
provide mockable access to globals.
Instead use services like `$window` and `$location` in functions on controllers, which are then called from expressions.
Such services provide mockable access to globals.

It is possible to access the context object using the identifier `this` and the locals object using the
identifier `$locals`.
@@ -30,42 +30,55 @@ so keeping to AngularJS standards is not just a functionality issue, it's also c
facilitate rapid security updates.


## Expression Sandboxing

AngularJS's expressions are sandboxed not for security reasons, but instead to maintain a proper
separation of application responsibilities. For example, access to `window` is disallowed
because it makes it easy to introduce brittle global state into your application.

However, this sandbox is not intended to stop attackers who can edit the template before it's
processed by Angular. It may be possible to run arbitrary JavaScript inside double-curly bindings
if an attacker can modify them.

But if an attacker can change arbitrary HTML templates, there's nothing stopping them from doing:

```html
<script>somethingEvil();</script>
```

**It's better to design your application in such a way that users cannot change client-side templates.**

For instance:
## Angular Templates and Expressions

**If an attacker has access to control Angular templates or expressions, they can exploit an Angular application
via an XSS attack, regardless of the version.**

There are a number of ways that templates and expressions can be controlled:

* **Generating Angular templates on the server containing user-provided content**. This is the most common pitfall
where you are generating HTML via some server-side engine such as PHP, Java or ASP.NET.
* **Passing an expression generated from user-provided content in calls to the following methods on a {@link scope scope}**:
* `$watch(userContent, ...)`
* `$watchGroup(userContent, ...)`
* `$watchCollection(userContent, ...)`
* `$eval(userContent)`
* `$evalAsync(userContent)`
* `$apply(userContent)`
* `$applyAsync(userContent)`
* **Passing an expression generated from user-provided content in calls to services that parse expressions**:
* `$compile(userContent)`
* `$parse(userContent)`
* `$interpolate(userContent)`
* **Passing an expression generated from user provided content as a predicate to `orderBy` pipe**:
`{{ value | orderBy : userContent }}`

### Sandbox removal
Each version of Angular 1 up to, but not including 1.6, contained an expression sandbox, which reduced the surface area of
the vulnerability but never removed it. **In Angular 1.6 we removed this sandbox as developers kept relying upon it as a security
feature even though it was always possible to access arbitrary JavaScript code if one could control the Angular templates
or expressions of applications.**

Control of the Angular templates makes applications vulnerable even if there was a completely secure sandbox:
* https://ryhanson.com/stealing-session-tokens-on-plunker-with-an-angular-expression-injection/ in this blog post the author shows
a (now closed) vulnerability in the Plunker application due to server-side rendering inside an Angular template.
* https://ryhanson.com/angular-expression-injection-walkthrough/ in this blog post the author describes an attack, which does not
rely upon an expression sandbox bypass, that can be made because the sample application is rendering a template on the server that
contains user entered content.

**It's best to design your application in such a way that users cannot change client-side templates.**

* Do not mix client and server templates
* Do not use user input to generate templates dynamically
* Do not run user input through `$scope.$eval`
* Do not run user input through `$scope.$eval` (or any of the other expression parsing functions listed above)
* Consider using {@link ng.directive:ngCsp CSP} (but don't rely only on CSP)

**You can use suitably sanitized server-side templating to dynamically generate CSS, URLs, etc, but not for generating templates that are
bootstrapped/compiled by Angular.**

### Mixing client-side and server-side templates

In general, we recommend against this because it can create unintended XSS vectors.

However, it's ok to mix server-side templating in the bootstrap template (`index.html`) as long
as user input cannot be used on the server to output html that would then be processed by Angular
in a way that would allow for arbitrary code execution.

**For instance, you can use server-side templating to dynamically generate CSS, URLs, etc, but not
for generating templates that are bootstrapped/compiled by Angular.**
**If you must continue to allow user-provided content in an Angular template then the safest option is to ensure that it is only
present in the part of the template that is made inert via the {@link ngNonBindable} directive.**


## HTTP Requests
Oops, something went wrong.

2 comments on commit 1547c75

@Narretz

This comment has been minimized.

Copy link
Contributor

Narretz replied Sep 15, 2016

As "refactor" it won't show up in the changelog, so we have to manually add it when generating

@petebacondarwin

This comment has been minimized.

Copy link
Member Author

petebacondarwin replied Sep 15, 2016

OK, good call @Narretz

Please sign in to comment.
You can’t perform that action at this time.