8000 refactor($parse): move around previous security changes made to $parse · blob42/angular.js@0af70eb · GitHub
[go: up one dir, main page]

Skip to content

Commit 0af70eb

Browse files
rodyhaddadIgorMinar
authored andcommitted
refactor($parse): move around previous security changes made to $parse
1 parent cb713e6 commit 0af70eb

File tree

3 files changed

+180
-249
lines changed

3 files changed

+180
-249
lines changed
Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,27 @@
11
@ngdoc error
22
@name $parse:isecfld
3-
@fullName Referencing 'constructor' Field in Expression
3+
@fullName Referencing Disallowed Field in Expression
44
@description
55

6-
Occurs when an expression attempts to access an objects constructor field.
6+
Occurs when an expression attempts to access one of the following fields:
77

8-
AngularJS bans constructor access from within expressions since constructor
9-
access is a known way to execute arbitrary Javascript code.
8+
* __proto__
9+
* __defineGetter__
10+
* __defineSetter__
11+
* __lookupGetter__
12+
* __lookupSetter__
1013

11-
To resolve this error, avoid constructor access. As a last resort, alias
12-
the constructor and access it through the alias instead.
14+
AngularJS bans access to these fields from within expressions since
15+
access is a known way to mess with native objects or
16+
to execute arbitrary Javascript code.
1317

14-
Example expression that would result in this error:
18+
To resolve this error, avoid using these fields in expressions. As a last resort,
19+
alias their value and access them through the alias instead.
20+
21+
Example expressions that would result in this error:
1522

1623
```
17-
<div>{{user.constructor.name}}</div>
18-
```
24+
<div>{{user.__proto__.hasOwnProperty = $emit}}</div>
25+
26+
<div>{{user.__defineGetter__('name', noop)}}</div>
27+
```

src/ng/parse.js

Lines changed: 25 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -12,39 +12,26 @@ var promiseWarning;
1212
//
1313
// As an example, consider the following Angular expression:
1414
//
15-
// {}.toString.constructor(alert("evil JS code"))
16-
//
17-
// We want to prevent this type of access. For the sake of performance, during the lexing phase we
18-
// disallow any "dotted" access to any member named "constructor".
19-
//
20-
// For reflective calls (a[b]) we check that the value of the lookup is not the Function constructor
21-
// while evaluating the expression, which is a stronger but more expensive test. Since reflective
22-
// calls are expensive anyway, this is not such a big deal compared to static dereferencing.
15+
// {}.toString.constructor('alert("evil JS code")')
2316
//
2417
// This sandboxing technique is not perfect and doesn't aim to be. The goal is to prevent exploits
2518
// against the expression language, but not to prevent exploits that were enabled by exposing
2619
// sensitive JavaScript or browser apis on Scope. Exposing such objects on a Scope is never a good
2720
// practice and therefore we are not even trying to protect against interaction with an object
2821
// explicitly exposed in this way.
2922
//
30-
// A developer could foil the name check by aliasing the Function constructor under a different
31-
// name on the scope.
32-
//
3323
// In general, it is not possible to access a Window object from an angular expression unless a
3424
// window or some DOM object that has a reference to window is published onto a Scope.
25+
// Similarly we prevent invocations of function known to be dangerous, as well as assignments to
26+
// native objects.
27+
3528

3629
function ensureSafeMemberName(name, fullExpression) {
37-
if (name === "constructor") {
30+
if (name === "__defineGetter__" || name === "__defineSetter__"
31+
|| name === "__lookupGetter__" || name === "__lookupSetter__"
32+
|| name === "__proto__") {
3833
throw $parseMinErr('isecfld',
39-
'Referencing "constructor" field in Angular expressions is disallowed! Expression: {0}',
40-
fullExpression);
41-
} else if (name === "__defineGetter__" || name === "__defineSetter__"
42-
|| name === "__lookupGetter__" || name === "__lookupSetter__") {
43-
throw $parseMinErr('isecgetset',
44-
'Defining and looking up getters and setters in Angular expressions is disallowed! '
45-
+'Expression: {0}', fullExpression);
46-
} else if (name === "__proto__") {
47-
throw $parseMinErr('isecproto', 'Using __proto__ in Angular expressions is disallowed! '
34+
'Attempting to access a disallowed field in Angular expressions! '
4835
+'Expression: {0}', fullExpression);
4936
}
5037
return name;
@@ -58,7 +45,7 @@ function ensureSafeObject(obj, fullExpression) {
5845
'Referencing Function in Angular expressions is disallowed! Expression: {0}',
5946
fullExpression);
6047
} else if (// isWindow(obj)
61-
obj.document && obj.location && obj.alert && obj.setInterval) {
48+
obj.window === obj) {
6249
throw $parseMinErr('isecwindow',
6350
'Referencing the Window in Angular expressions is disallowed! Expression: {0}',
6451
fullExpression);
@@ -67,21 +54,26 @@ function ensureSafeObject(obj, fullExpression) {
6754
throw $parseMinErr('isecdom',
6855
'Referencing DOM nodes in Angular expressions is disallowed! Expression: {0}',
6956
fullExpression);
70-
} else if (// isObject(obj)
71-
obj.getOwnPropertyNames || obj.getOwnPropertyDescriptor) {
57+
} else if (// block Object so that we can't get hold of dangerous Object.* methods
58+
obj === Object) {
7259
throw $parseMinErr('isecobj',
7360
'Referencing Object in Angular expressions is disallowed! Expression: {0}',
7461
fullExpression);
75-
} else if (obj === ({}).__defineGetter__ || obj === ({}).__defineSetter__
76-
|| obj === ({}).__lookupGetter__ || obj === ({}).__lookupSetter__) {
77-
throw $parseMinErr('isecgetset',
78-
'Defining and looking up getters and setters in Angular expressions is disallowed! '
79-
+'Expression: {0}', fullExpression);
8062
}
8163
}
8264
return obj;
8365
}
8466

67+
function ensureSafeFunction(obj, fullExpression) {
68+
if (obj) {
69+
if (obj.constructor === obj) {
70+
throw $parseMinErr('isecfn',
71+
'Referencing Function in Angular expressions is disallowed! Expression: {0}',
72+
fullExpression);
73+
}
74+
}
75+
}
76+
8577
var OPERATORS = {
8678
/* jshint bitwise : false */
8779
'null':function(){return null;},
@@ -716,10 +708,7 @@ Parser.prototype = {
716708
i = indexFn(self, locals),
717709
v, p;
718710

719-
if (i === "__proto__") {
720-
throw $parseMinErr('isecproto', 'Using __proto__ in Angular expressions is disallowed! '
721-
+'Expression: {0}', parser.text);
722-
}
711+
ensureSafeMemberName(i, parser.text);
723712
if (!o) return undefined;
724713
v = ensureSafeObject(o[i], parser.text);
725714
if (v && v.then && parser.options.unwrapPromises) {
@@ -762,7 +751,7 @@ Parser.prototype = {
762751
var fnPtr = fn(scope, locals, context) || noop;
763752

764753
ensureSafeObject(context, parser.text);
765-
ensureSafeObject(fnPtr, parser.text);
754+
ensureSafeFunction(fnPtr, parser.text);
766755

767756
// IE stupidity! (IE doesn't have apply for some native functions)
768757
var v = fnPtr.apply
@@ -871,6 +860,8 @@ function setter(obj, path, setValue, fullExp, options) {
871860
}
872861
}
873862
key = ensureSafeMemberName(element.shift(), fullExp);
863+
ensureSafeObject(obj, fullExp);
864+
ensureSafeObject(obj[key], fullExp);
874865
obj[key] = setValue;
875866
return setValue;
876867
}

0 commit comments

Comments
 (0)
0