aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/ng/parse.js87
-rw-r--r--test/ng/parseSpec.js158
2 files changed, 216 insertions, 29 deletions
diff --git a/src/ng/parse.js b/src/ng/parse.js
index 28f69a60..102b3312 100644
--- a/src/ng/parse.js
+++ b/src/ng/parse.js
@@ -1,5 +1,54 @@
'use strict';
+var $parseMinErr = minErr('$parse');
+
+// Sandboxing Angular Expressions
+// ------------------------------
+// Angular expressions are generally considered safe because these expressions only have direct access to $scope and
+// locals. However, one can obtain the ability to execute arbitrary JS code by obtaining a reference to native JS
+// functions such as the Function constructor.
+//
+// As an example, consider the following Angular expression:
+//
+// {}.toString.constructor(alert("evil JS code"))
+//
+// We want to prevent this type of access. For the sake of performance, during the lexing phase we disallow any "dotted"
+// access to any member named "constructor".
+//
+// For reflective calls (a[b]) we check that the value of the lookup is not the Function constructor while evaluating
+// For reflective calls (a[b]) we check that the value of the lookup is not the Function constructor while evaluating
+// the expression, which is a stronger but more expensive test. Since reflective calls are expensive anyway, this is not
+// such a big deal compared to static dereferencing.
+//
+// This sandboxing techniqueue is not perfect and doesn't aim to be. The goal is to prevent exploits against the
+// expression language, but not to prevent exploits that were enabled by exposing sensitive JavaScript or browser apis
+// on Scope. Exposing such objects on a Scope is never a good practice and therefore we are not even trying to protect
+// against interaction with an object explicitly exposed in this way.
+//
+// A developer could foil the name check by aliasing the Function constructor under a different name on the scope.
+//
+// In general, it is not possible to access a Window object from an angular expression unless a window or some DOM
+// object that has a reference to window is published onto a Scope.
+
+function ensureSafeMemberName(name, fullExpression) {
+ if (name === "constructor") {
+ throw $parseMinErr('isecfld',
+ 'Referencing "constructor" field in Angular expressions is disallowed! Expression: {0}', fullExpression);
+ }
+ return name;
+};
+
+function ensureSafeObject(obj, fullExpression) {
+ // nifty check if obj is Function that is fast and works across iframes and other contexts
+ if (obj && obj.constructor === obj) {
+ throw $parseMinErr('isecfn',
+ 'Referencing Function in Angular expressions is disallowed! Expression: {0}', fullExpression);
+ } else {
+ return obj;
+ }
+}
+
+
var OPERATORS = {
'null':function(){return null;},
'true':function(){return true;},
@@ -36,7 +85,6 @@ var OPERATORS = {
'!':function(self, locals, a){return !a(self, locals);}
};
var ESCAPE = {"n":"\n", "f":"\f", "r":"\r", "t":"\t", "v":"\v", "'":"'", '"':'"'};
-var $parseMinErr = minErr('$parse');
function lex(text, csp){
var tokens = [],
@@ -204,12 +252,12 @@ function lex(text, csp){
if (OPERATORS.hasOwnProperty(ident)) {
token.fn = token.json = OPERATORS[ident];
} else {
- var getter = getterFn(ident, csp);
+ var getter = getterFn(ident, csp, text);
token.fn = extend(function(self, locals) {
return (getter(self, locals));
}, {
assign: function(self, value) {
- return setter(self, ident, value);
+ return setter(self, ident, value, text);
}
});
}
@@ -583,14 +631,14 @@ function parser(text, json, $filter, csp){
function _fieldAccess(object) {
var field = expect().text;
- var getter = getterFn(field, csp);
+ var getter = getterFn(field, csp, text);
return extend(
function(scope, locals, self) {
return getter(self || object(scope, locals), locals);
},
{
assign:function(scope, value, locals) {
- return setter(object(scope, locals), field, value);
+ return setter(object(scope, locals), field, value, text);
}
}
);
@@ -606,7 +654,7 @@ function parser(text, json, $filter, csp){
v, p;
if (!o) return undefined;
- v = o[i];
+ v = ensureSafeObject(o[i], text);
if (v && v.then) {
p = v;
if (!('$$v' in v)) {
@@ -618,7 +666,9 @@ function parser(text, json, $filter, csp){
return v;
}, {
assign:function(self, value, locals){
- return obj(self, locals)[indexFn(self, locals)] = value;
+ var key = indexFn(self, locals);
+ // prevent overwriting of Function.constructor which would break ensureSafeObject check
+ return ensureSafeObject(obj(self, locals), text)[key] = value;
}
});
}
@@ -706,10 +756,10 @@ function parser(text, json, $filter, csp){
// Parser helper functions
//////////////////////////////////////////////////
-function setter(obj, path, setValue) {
- var element = path.split('.');
+function setter(obj, path, setValue, fullExp) {
+ var element = path.split('.'), key;
for (var i = 0; element.length > 1; i++) {
- var key = element.shift();
+ key = ensureSafeMemberName(element.shift(), fullExp);
var propertyObj = obj[key];
if (!propertyObj) {
propertyObj = {};
@@ -717,7 +767,8 @@ function setter(obj, path, setValue) {
}
obj = propertyObj;
}
- obj[element.shift()] = setValue;
+ key = ensureSafeMemberName(element.shift(), fullExp);
+ obj[key] = setValue;
return setValue;
}
@@ -728,7 +779,12 @@ var getterFnCache = {};
* - http://jsperf.com/angularjs-parse-getter/4
* - http://jsperf.com/path-evaluation-simplified/7
*/
-function cspSafeGetterFn(key0, key1, key2, key3, key4) {
+function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) {
+ ensureSafeMemberName(key0, fullExp);
+ ensureSafeMemberName(key1, fullExp);
+ ensureSafeMemberName(key2, fullExp);
+ ensureSafeMemberName(key3, fullExp);
+ ensureSafeMemberName(key4, fullExp);
return function(scope, locals) {
var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope,
promise;
@@ -792,7 +848,7 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4) {
};
}
-function getterFn(path, csp) {
+function getterFn(path, csp, fullExp) {
if (getterFnCache.hasOwnProperty(path)) {
return getterFnCache[path];
}
@@ -803,12 +859,12 @@ function getterFn(path, csp) {
if (csp) {
fn = (pathKeysLength < 6)
- ? cspSafeGetterFn(pathKeys[0], pathKeys[1], pathKeys[2], pathKeys[3], pathKeys[4])
+ ? cspSafeGetterFn(pathKeys[0], pathKeys[1], pathKeys[2], pathKeys[3], pathKeys[4], fullExp)
: function(scope, locals) {
var i = 0, val;
do {
val = cspSafeGetterFn(
- pathKeys[i++], pathKeys[i++], pathKeys[i++], pathKeys[i++], pathKeys[i++]
+ pathKeys[i++], pathKeys[i++], pathKeys[i++], pathKeys[i++], pathKeys[i++], fullExp
)(scope, locals);
locals = undefined; // clear after first iteration
@@ -819,6 +875,7 @@ function getterFn(path, csp) {
} else {
var code = 'var l, fn, p;\n';
forEach(pathKeys, function(key, index) {
+ ensureSafeMemberName(key, fullExp);
code += 'if(s === null || s === undefined) return s;\n' +
'l=s;\n' +
's='+ (index
diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js
index f73a6021..62425f12 100644
--- a/test/ng/parseSpec.js
+++ b/test/ng/parseSpec.js
@@ -103,7 +103,7 @@ describe('parser', function() {
expect(tokens[7].text).toEqual('===');
expect(tokens[8].text).toEqual('!==');
});
-
+
it('should tokenize logical and ternary', function() {
var tokens = lex("&& || ? :");
expect(tokens[0].text).toEqual('&&');
@@ -228,7 +228,7 @@ describe('parser', function() {
expect(scope.$eval("0||2")).toEqual(0||2);
expect(scope.$eval("0||1&&2")).toEqual(0||1&&2);
});
-
+
it('should parse ternary', function(){
var returnTrue = scope.returnTrue = function(){ return true; };
var returnFalse = scope.returnFalse = function(){ return false; };
@@ -239,7 +239,7 @@ describe('parser', function() {
// Simple.
expect(scope.$eval('0?0:2')).toEqual(0?0:2);
expect(scope.$eval('1?0:2')).toEqual(1?0:2);
-
+
// Nested on the left.
expect(scope.$eval('0?0?0:0:2')).toEqual(0?0?0:0:2);
expect(scope.$eval('1?0?0:0:2')).toEqual(1?0?0:0:2);
@@ -250,7 +250,7 @@ describe('parser', function() {
expect(scope.$eval('1?1?1:0:2')).toEqual(1?1?1:0:2);
expect(scope.$eval('1?1?1:2:3')).toEqual(1?1?1:2:3);
expect(scope.$eval('1?1?1:2:3')).toEqual(1?1?1:2:3);
-
+
// Nested on the right.
expect(scope.$eval('0?0:0?0:2')).toEqual(0?0:0?0:2);
expect(scope.$eval('1?0:0?0:2')).toEqual(1?0:0?0:2);
@@ -261,31 +261,31 @@ describe('parser', function() {
expect(scope.$eval('1?1:1?0:2')).toEqual(1?1:1?0:2);
expect(scope.$eval('1?1:1?2:3')).toEqual(1?1:1?2:3);
expect(scope.$eval('1?1:1?2:3')).toEqual(1?1:1?2:3);
-
+
// Precedence with respect to logical operators.
expect(scope.$eval('0&&1?0:1')).toEqual(0&&1?0:1);
expect(scope.$eval('1||0?0:0')).toEqual(1||0?0:0);
-
+
expect(scope.$eval('0?0&&1:2')).toEqual(0?0&&1:2);
expect(scope.$eval('0?1&&1:2')).toEqual(0?1&&1:2);
expect(scope.$eval('0?0||0:1')).toEqual(0?0||0:1);
expect(scope.$eval('0?0||1:2')).toEqual(0?0||1:2);
-
+
expect(scope.$eval('1?0&&1:2')).toEqual(1?0&&1:2);
expect(scope.$eval('1?1&&1:2')).toEqual(1?1&&1:2);
expect(scope.$eval('1?0||0:1')).toEqual(1?0||0:1);
expect(scope.$eval('1?0||1:2')).toEqual(1?0||1:2);
-
+
expect(scope.$eval('0?1:0&&1')).toEqual(0?1:0&&1);
expect(scope.$eval('0?2:1&&1')).toEqual(0?2:1&&1);
expect(scope.$eval('0?1:0||0')).toEqual(0?1:0||0);
expect(scope.$eval('0?2:0||1')).toEqual(0?2:0||1);
-
+
expect(scope.$eval('1?1:0&&1')).toEqual(1?1:0&&1);
expect(scope.$eval('1?2:1&&1')).toEqual(1?2:1&&1);
expect(scope.$eval('1?1:0||0')).toEqual(1?1:0||0);
expect(scope.$eval('1?2:0||1')).toEqual(1?2:0||1);
-
+
// Function calls.
expect(scope.$eval('returnTrue() ? returnString() : returnInt()')).toEqual(returnTrue() ? returnString() : returnInt());
expect(scope.$eval('returnFalse() ? returnString() : returnInt()')).toEqual(returnFalse() ? returnString() : returnInt());
@@ -334,12 +334,12 @@ describe('parser', function() {
it('should support property names that collide with native object properties', function() {
// regression
scope.watch = 1;
- scope.constructor = 2;
- scope.toString = 3;
+ scope.toString = function toString() {
+ return "custom toString";
+ };
expect(scope.$eval('watch', scope)).toBe(1);
- expect(scope.$eval('constructor', scope)).toBe(2);
- expect(scope.$eval('toString', scope)).toBe(3);
+ expect(scope.$eval('toString()', scope)).toBe('custom toString');
});
it('should evaluate grouped expressions', function() {
@@ -554,6 +554,136 @@ describe('parser', function() {
expect(scope.$eval('bool.toString()')).toBe('false');
});
+ describe('sandboxing', function() {
+ it('should NOT allow access to Function constructor in getter', function() {
+ expect(function() {
+ scope.$eval('{}.toString.constructor');
+ }).toThrow(new Error(
+ '[$parse:isecfld] Referencing "constructor" field in Angular expressions is disallowed! ' +
+ 'Expression: {}.toString.constructor'));
+
+ expect(function() {
+ scope.$eval('{}.toString.constructor("alert(1)")');
+ }).toThrow(new Error(
+ '[$parse:isecfld] Referencing "constructor" field in Angular expressions is disallowed! ' +
+ 'Expression: {}.toString.constructor("alert(1)")'));
+
+ expect(function() {
+ scope.$eval('[].toString.constructor.foo');
+ }).toThrow(new Error(
+ '[$parse:isecfld] Referencing "constructor" field in Angular expressions is disallowed! ' +
+ 'Expression: [].toString.constructor.foo'));
+
+ expect(function() {
+ scope.$eval('{}.toString["constructor"]');
+ }).toThrow(new Error(
+ '[$parse:isecfn] Referencing Function in Angular expressions is disallowed! ' +
+ 'Expression: {}.toString["constructor"]'));
+ expect(function() {
+ scope.$eval('{}["toString"]["constructor"]');
+ }).toThrow(new Error(
+ '[$parse:isecfn] Referencing Function in Angular expressions is disallowed! ' +
+ 'Expression: {}["toString"]["constructor"]'));
+
+ scope.a = [];
+ expect(function() {
+ scope.$eval('a.toString.constructor', scope);
+ }).toThrow(new Error(
+ '[$parse:isecfld] Referencing "constructor" field in Angular expressions is disallowed! ' +
+ 'Expression: a.toString.constructor'));
+ expect(function() {
+ scope.$eval('a.toString["constructor"]', scope);
+ }).toThrow(new Error(
+ '[$parse:isecfn] Referencing Function in Angular expressions is disallowed! ' +
+ 'Expression: a.toString["constructor"]'));
+ });
+
+ it('should NOT allow access to Function constructor in setter', function() {
+ expect(function() {
+ scope.$eval('{}.toString.constructor = 1');
+ }).toThrow(new Error(
+ '[$parse:isecfld] Referencing "constructor" field in Angular expressions is disallowed! ' +
+ 'Expression: {}.toString.constructor = 1'));
+
+ expect(function() {
+ scope.$eval('{}.toString.constructor.a = 1');
+ }).toThrow(new Error(
+ '[$parse:isecfld] Referencing "constructor" field in Angular expressions is disallowed! ' +
+ 'Expression: {}.toString.constructor.a = 1'));
+
+ expect(function() {
+ scope.$eval('{}.toString["constructor"]["constructor"] = 1');
+ }).toThrow(new Error(
+ '[$parse:isecfn] Referencing Function in Angular expressions is disallowed! ' +
+ 'Expression: {}.toString["constructor"]["constructor"] = 1'));
+
+
+ scope.key1 = "const";
+ scope.key2 = "ructor";
+ expect(function() {
+ scope.$eval('{}.toString[key1 + key2].foo = 1');
+ }).toThrow(new Error(
+ '[$parse:isecfn] Referencing Function in Angular expressions is disallowed! ' +
+ 'Expression: {}.toString[key1 + key2].foo = 1'));
+
+ expect(function() {
+ scope.$eval('{}.toString["constructor"]["a"] = 1');
+ }).toThrow(new Error(
+ '[$parse:isecfn] Referencing Function in Angular expressions is disallowed! ' +
+ 'Expression: {}.toString["constructor"]["a"] = 1'));
+
+ scope.a = [];
+ expect(function() {
+ scope.$eval('a.toString.constructor = 1', scope);
+ }).toThrow(new Error(
+ '[$parse:isecfld] Referencing "constructor" field in Angular expressions is disallowed! ' +
+ 'Expression: a.toString.constructor = 1'));
+ });
+
+
+ it('should NOT allow access to Function constructor that has been aliased', function() {
+ scope.foo = { "bar": Function };
+ expect(function() {
+ scope.$eval('foo["bar"]');
+ }).toThrow(new Error(
+ '[$parse:isecfn] Referencing Function in Angular expressions is disallowed! ' +
+ 'Expression: foo["bar"]'));
+
+ });
+ });
+
+ describe('overriding constructor', function() {
+ it('should evaluate grouped expressions', function() {
+ scope.foo = function foo() {
+ return "foo";
+ };
+ // When not overridden, access should be restricted both by the dot operator and by the
+ // index operator.
+ expect(function() {
+ scope.$eval('foo.constructor()', scope)
+ }).toThrow(new Error(
+ '[$parse:isecfld] Referencing "constructor" field in Angular expressions is disallowed! ' +
+ 'Expression: foo.constructor()'));
+ expect(function() {
+ scope.$eval('foo["constructor"]()', scope)
+ }).toThrow(new Error(
+ '[$parse:isecfn] Referencing Function in Angular expressions is disallowed! ' +
+ 'Expression: foo["constructor"]()'));
+
+ // User defined value assigned to constructor.
+ scope.foo.constructor = function constructor() {
+ return "custom constructor";
+ }
+ // Dot operator should still block it.
+ expect(function() {
+ scope.$eval('foo.constructor()', scope)
+ }).toThrow(new Error(
+ '[$parse:isecfld] Referencing "constructor" field in Angular expressions is disallowed! ' +
+ 'Expression: foo.constructor()'));
+ // However, the index operator should allow it.
+ expect(scope.$eval('foo["constructor"]()', scope)).toBe('custom constructor');
+ });
+ });
it('should call the function from the received instance and not from a new one', function() {
var n = 0;
generated by cgit v1.2.3 (git 2.25.1) at 2025-11-01 10:42:18 +0000