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

Commit 1547c75

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
1 parent 76d3daf commit 1547c75

File tree

12 files changed

+66
-1222
lines changed

12 files changed

+66
-1222
lines changed

docs/content/error/$parse/isecaf.ngdoc

-12
This file was deleted.

docs/content/error/$parse/isecdom.ngdoc

-47
This file was deleted.

docs/content/error/$parse/isecff.ngdoc

-17
This file was deleted.

docs/content/error/$parse/isecfld.ngdoc

-27
This file was deleted.

docs/content/error/$parse/isecfn.ngdoc

-10
This file was deleted.

docs/content/error/$parse/isecobj.ngdoc

-11
This file was deleted.

docs/content/error/$parse/isecwindow.ngdoc

-45
This file was deleted.

docs/content/guide/expression.ngdoc

+3-3
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,11 @@ You can try evaluating different expressions here:
113113
Angular does not use JavaScript's `eval()` to evaluate expressions. Instead Angular's
114114
{@link ng.$parse $parse} service processes these expressions.
115115

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

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

122122
It is possible to access the context object using the identifier `this` and the locals object using the
123123
identifier `$locals`.

docs/content/guide/security.ngdoc

+43-30
Original file line numberDiff line numberDiff line change
@@ -30,42 +30,55 @@ so keeping to AngularJS standards is not just a functionality issue, it's also c
3030
facilitate rapid security updates.
3131

3232

33-
## Expression Sandboxing
34-
35-
AngularJS's expressions are sandboxed not for security reasons, but instead to maintain a proper
36-
separation of application responsibilities. For example, access to `window` is disallowed
37-
because it makes it easy to introduce brittle global state into your application.
38-
39-
However, this sandbox is not intended to stop attackers who can edit the template before it's
40-
processed by Angular. It may be possible to run arbitrary JavaScript inside double-curly bindings
41-
if an attacker can modify them.
42-
43-
But if an attacker can change arbitrary HTML templates, there's nothing stopping them from doing:
44-
45-
```html
46-
<script>somethingEvil();</script>
47-
```
48-
49-
**It's better to design your application in such a way that users cannot change client-side templates.**
50-
51-
For instance:
33+
## Angular Templates and Expressions
34+
35+
**If an attacker has access to control Angular templates or expressions, they can exploit an Angular application
36+
via an XSS attack, regardless of the version.**
37+
38+
There are a number of ways that templates and expressions can be controlled:
39+
40+
* **Generating Angular templates on the server containing user-provided content**. This is the most common pitfall
41+
where you are generating HTML via some server-side engine such as PHP, Java or ASP.NET.
42+
* **Passing an expression generated from user-provided content in calls to the following methods on a {@link scope scope}**:
43+
* `$watch(userContent, ...)`
44+
* `$watchGroup(userContent, ...)`
45+
* `$watchCollection(userContent, ...)`
46+
* `$eval(userContent)`
47+
* `$evalAsync(userContent)`
48+
* `$apply(userContent)`
49+
* `$applyAsync(userContent)`
50+
* **Passing an expression generated from user-provided content in calls to services that parse expressions**:
51+
* `$compile(userContent)`
52+
* `$parse(userContent)`
53+
* `$interpolate(userContent)`
54+
* **Passing an expression generated from user provided content as a predicate to `orderBy` pipe**:
55+
`{{ value | orderBy : userContent }}`
56+
57+
### Sandbox removal
58+
Each version of Angular 1 up to, but not including 1.6, contained an expression sandbox, which reduced the surface area of
59+
the vulnerability but never removed it. **In Angular 1.6 we removed this sandbox as developers kept relying upon it as a security
60+
feature even though it was always possible to access arbitrary JavaScript code if one could control the Angular templates
61+
or expressions of applications.**
62+
63+
Control of the Angular templates makes applications vulnerable even if there was a completely secure sandbox:
64+
* https://ryhanson.com/stealing-session-tokens-on-plunker-with-an-angular-expression-injection/ in this blog post the author shows
65+
a (now closed) vulnerability in the Plunker application due to server-side rendering inside an Angular template.
66+
* https://ryhanson.com/angular-expression-injection-walkthrough/ in this blog post the author describes an attack, which does not
67+
rely upon an expression sandbox bypass, that can be made because the sample application is rendering a template on the server that
68+
contains user entered content.
69+
70+
**It's best to design your application in such a way that users cannot change client-side templates.**
5271

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

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

59-
### Mixing client-side and server-side templates
60-
61-
In general, we recommend against this because it can create unintended XSS vectors.
62-
63-
However, it's ok to mix server-side templating in the bootstrap template (`index.html`) as long
64-
as user input cannot be used on the server to output html that would then be processed by Angular
65-
in a way that would allow for arbitrary code execution.
66-
67-
**For instance, you can use server-side templating to dynamically generate CSS, URLs, etc, but not
68-
for generating templates that are bootstrapped/compiled by Angular.**
80+
**If you must continue to allow user-provided content in an Angular template then the safest option is to ensure that it is only
81+
present in the part of the template that is made inert via the {@link ngNonBindable} directive.**
6982

7083

7184
## HTTP Requests

0 commit comments

Comments
 (0)