fix($parse): disallow access to Function constructor

Enhances sandboxing of Angular Expressions to prevent attacks via:

  {}.toString.constructor(alert("evil JS code"))
This commit is contained in:
Chirayu Krishnappa 2013-06-24 14:14:54 -07:00 committed by Igor Minar
parent fd87eb0ca5
commit 5349b20097
2 changed files with 216 additions and 29 deletions

View file

@ -1,5 +1,54 @@
'use strict'; '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 = { var OPERATORS = {
'null':function(){return null;}, 'null':function(){return null;},
'true':function(){return true;}, 'true':function(){return true;},
@ -36,7 +85,6 @@ var OPERATORS = {
'!':function(self, locals, a){return !a(self, locals);} '!':function(self, locals, a){return !a(self, locals);}
}; };
var ESCAPE = {"n":"\n", "f":"\f", "r":"\r", "t":"\t", "v":"\v", "'":"'", '"':'"'}; var ESCAPE = {"n":"\n", "f":"\f", "r":"\r", "t":"\t", "v":"\v", "'":"'", '"':'"'};
var $parseMinErr = minErr('$parse');
function lex(text, csp){ function lex(text, csp){
var tokens = [], var tokens = [],
@ -204,12 +252,12 @@ function lex(text, csp){
if (OPERATORS.hasOwnProperty(ident)) { if (OPERATORS.hasOwnProperty(ident)) {
token.fn = token.json = OPERATORS[ident]; token.fn = token.json = OPERATORS[ident];
} else { } else {
var getter = getterFn(ident, csp); var getter = getterFn(ident, csp, text);
token.fn = extend(function(self, locals) { token.fn = extend(function(self, locals) {
return (getter(self, locals)); return (getter(self, locals));
}, { }, {
assign: function(self, value) { 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) { function _fieldAccess(object) {
var field = expect().text; var field = expect().text;
var getter = getterFn(field, csp); var getter = getterFn(field, csp, text);
return extend( return extend(
function(scope, locals, self) { function(scope, locals, self) {
return getter(self || object(scope, locals), locals); return getter(self || object(scope, locals), locals);
}, },
{ {
assign:function(scope, value, 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; v, p;
if (!o) return undefined; if (!o) return undefined;
v = o[i]; v = ensureSafeObject(o[i], text);
if (v && v.then) { if (v && v.then) {
p = v; p = v;
if (!('$$v' in v)) { if (!('$$v' in v)) {
@ -618,7 +666,9 @@ function parser(text, json, $filter, csp){
return v; return v;
}, { }, {
assign:function(self, value, locals){ 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 // Parser helper functions
////////////////////////////////////////////////// //////////////////////////////////////////////////
function setter(obj, path, setValue) { function setter(obj, path, setValue, fullExp) {
var element = path.split('.'); var element = path.split('.'), key;
for (var i = 0; element.length > 1; i++) { for (var i = 0; element.length > 1; i++) {
var key = element.shift(); key = ensureSafeMemberName(element.shift(), fullExp);
var propertyObj = obj[key]; var propertyObj = obj[key];
if (!propertyObj) { if (!propertyObj) {
propertyObj = {}; propertyObj = {};
@ -717,7 +767,8 @@ function setter(obj, path, setValue) {
} }
obj = propertyObj; obj = propertyObj;
} }
obj[element.shift()] = setValue; key = ensureSafeMemberName(element.shift(), fullExp);
obj[key] = setValue;
return setValue; return setValue;
} }
@ -728,7 +779,12 @@ var getterFnCache = {};
* - http://jsperf.com/angularjs-parse-getter/4 * - http://jsperf.com/angularjs-parse-getter/4
* - http://jsperf.com/path-evaluation-simplified/7 * - 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) { return function(scope, locals) {
var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope, var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope,
promise; 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)) { if (getterFnCache.hasOwnProperty(path)) {
return getterFnCache[path]; return getterFnCache[path];
} }
@ -803,12 +859,12 @@ function getterFn(path, csp) {
if (csp) { if (csp) {
fn = (pathKeysLength < 6) 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) { : function(scope, locals) {
var i = 0, val; var i = 0, val;
do { do {
val = cspSafeGetterFn( 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); )(scope, locals);
locals = undefined; // clear after first iteration locals = undefined; // clear after first iteration
@ -819,6 +875,7 @@ function getterFn(path, csp) {
} else { } else {
var code = 'var l, fn, p;\n'; var code = 'var l, fn, p;\n';
forEach(pathKeys, function(key, index) { forEach(pathKeys, function(key, index) {
ensureSafeMemberName(key, fullExp);
code += 'if(s === null || s === undefined) return s;\n' + code += 'if(s === null || s === undefined) return s;\n' +
'l=s;\n' + 'l=s;\n' +
's='+ (index 's='+ (index

View file

@ -103,7 +103,7 @@ describe('parser', function() {
expect(tokens[7].text).toEqual('==='); expect(tokens[7].text).toEqual('===');
expect(tokens[8].text).toEqual('!=='); expect(tokens[8].text).toEqual('!==');
}); });
it('should tokenize logical and ternary', function() { it('should tokenize logical and ternary', function() {
var tokens = lex("&& || ? :"); var tokens = lex("&& || ? :");
expect(tokens[0].text).toEqual('&&'); expect(tokens[0].text).toEqual('&&');
@ -228,7 +228,7 @@ describe('parser', function() {
expect(scope.$eval("0||2")).toEqual(0||2); expect(scope.$eval("0||2")).toEqual(0||2);
expect(scope.$eval("0||1&&2")).toEqual(0||1&&2); expect(scope.$eval("0||1&&2")).toEqual(0||1&&2);
}); });
it('should parse ternary', function(){ it('should parse ternary', function(){
var returnTrue = scope.returnTrue = function(){ return true; }; var returnTrue = scope.returnTrue = function(){ return true; };
var returnFalse = scope.returnFalse = function(){ return false; }; var returnFalse = scope.returnFalse = function(){ return false; };
@ -239,7 +239,7 @@ describe('parser', function() {
// Simple. // Simple.
expect(scope.$eval('0?0:2')).toEqual(0?0:2); expect(scope.$eval('0?0:2')).toEqual(0?0:2);
expect(scope.$eval('1?0:2')).toEqual(1?0:2); expect(scope.$eval('1?0:2')).toEqual(1?0:2);
// Nested on the left. // Nested on the left.
expect(scope.$eval('0?0?0:0:2')).toEqual(0?0?0:0:2); 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); 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: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);
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. // Nested on the right.
expect(scope.$eval('0?0:0?0:2')).toEqual(0?0:0?0:2); 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); 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?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);
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. // Precedence with respect to logical operators.
expect(scope.$eval('0&&1?0:1')).toEqual(0&&1?0:1); 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('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?0&&1:2')).toEqual(0?0&&1:2);
expect(scope.$eval('0?1&&1:2')).toEqual(0?1&&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||0:1')).toEqual(0?0||0:1);
expect(scope.$eval('0?0||1:2')).toEqual(0?0||1:2); 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?0&&1:2')).toEqual(1?0&&1:2);
expect(scope.$eval('1?1&&1:2')).toEqual(1?1&&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||0:1')).toEqual(1?0||0:1);
expect(scope.$eval('1?0||1:2')).toEqual(1?0||1:2); 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?1:0&&1')).toEqual(0?1:0&&1);
expect(scope.$eval('0?2:1&&1')).toEqual(0?2:1&&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?1:0||0')).toEqual(0?1:0||0);
expect(scope.$eval('0?2:0||1')).toEqual(0?2:0||1); 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?1:0&&1')).toEqual(1?1:0&&1);
expect(scope.$eval('1?2:1&&1')).toEqual(1?2:1&&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?1:0||0')).toEqual(1?1:0||0);
expect(scope.$eval('1?2:0||1')).toEqual(1?2:0||1); expect(scope.$eval('1?2:0||1')).toEqual(1?2:0||1);
// Function calls. // Function calls.
expect(scope.$eval('returnTrue() ? returnString() : returnInt()')).toEqual(returnTrue() ? returnString() : returnInt()); expect(scope.$eval('returnTrue() ? returnString() : returnInt()')).toEqual(returnTrue() ? returnString() : returnInt());
expect(scope.$eval('returnFalse() ? returnString() : returnInt()')).toEqual(returnFalse() ? 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() { it('should support property names that collide with native object properties', function() {
// regression // regression
scope.watch = 1; scope.watch = 1;
scope.constructor = 2; scope.toString = function toString() {
scope.toString = 3; return "custom toString";
};
expect(scope.$eval('watch', scope)).toBe(1); expect(scope.$eval('watch', scope)).toBe(1);
expect(scope.$eval('constructor', scope)).toBe(2); expect(scope.$eval('toString()', scope)).toBe('custom toString');
expect(scope.$eval('toString', scope)).toBe(3);
}); });
it('should evaluate grouped expressions', function() { it('should evaluate grouped expressions', function() {
@ -554,6 +554,136 @@ describe('parser', function() {
expect(scope.$eval('bool.toString()')).toBe('false'); 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() { it('should call the function from the received instance and not from a new one', function() {
var n = 0; var n = 0;