fix($parse): add quick check for Function constructor in fast path

This commit is contained in:
Chirayu Krishnappa
2014-11-03 19:14:58 -08:00
parent 288b531626
commit e676d642f5
4 changed files with 73 additions and 11 deletions

View File

@@ -71,7 +71,8 @@ app.controller('DataController', function($scope, $rootScope) {
date2: new Date(Math.random()*Date.now()),
func: function(){ return star; },
obj: data[i-1],
keys: data[i-1] && (data[i-1].keys || Object.keys(data[i-1]))
keys: data[i-1] && (data[i-1].keys || Object.keys(data[i-1])),
constructor: data[i-1]
});
}

View File

@@ -16,6 +16,12 @@
<label for="complexPath">Complex Paths</label>
</li>
<li>
<input type="radio" ng-model="expressionType" value="constructorPath" id="constructorPath">
<label for="constructorPath">Constructor Paths</label>
($parse special cases "constructor" for security)
</li>
<li>
<input type="radio" ng-model="expressionType" value="fieldAccess" id="fieldAccess">
<label for="fieldAccess">Field Accessors</label>
@@ -77,6 +83,17 @@
<span bm-pe-watch="row.keys"></span>
</li>
<li ng-switch-when="constructorPath" ng-repeat="(rowIdx, row) in ::data">
<span bm-pe-watch="row.index"></span>
<span bm-pe-watch="row.constructor.index"></span>
<span bm-pe-watch="row.constructor.index"></span>
<span bm-pe-watch="row.constructor.index"></span>
<span bm-pe-watch="row.constructor.constructor.index"></span>
<span bm-pe-watch="row.constructor.constructor.index"></span>
<span bm-pe-watch="row.constructor.constructor.constructor.index"></span>
<span bm-pe-watch="row.constructor.constructor.constructor.index"></span>
</li>
<li ng-switch-when="complexPath" ng-repeat="(rowIdx, row) in ::data">
<span bm-pe-watch="row.index"></span>
<span bm-pe-watch="row.num0"></span>

View File

@@ -854,6 +854,10 @@ function setter(obj, path, setValue, fullExp) {
var getterFnCache = createMap();
function isPossiblyDangerousMemberName(name) {
return name == 'constructor';
}
/**
* Implementation of the "Black Hole" variant from:
* - http://jsperf.com/angularjs-parse-getter/4
@@ -865,33 +869,47 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) {
ensureSafeMemberName(key2, fullExp);
ensureSafeMemberName(key3, fullExp);
ensureSafeMemberName(key4, fullExp);
var eso = function(o) {
return ensureSafeObject(o, fullExp);
};
var eso0 = isPossiblyDangerousMemberName(key0) ? eso : identity;
var eso1 = isPossiblyDangerousMemberName(key1) ? eso : identity;
var eso2 = isPossiblyDangerousMemberName(key2) ? eso : identity;
var eso3 = isPossiblyDangerousMemberName(key3) ? eso : identity;
var eso4 = isPossiblyDangerousMemberName(key4) ? eso : identity;
return function cspSafeGetter(scope, locals) {
var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope;
if (pathVal == null) return pathVal;
pathVal = pathVal[key0];
pathVal = eso0(pathVal[key0]);
if (!key1) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key1];
pathVal = eso1(pathVal[key1]);
if (!key2) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key2];
pathVal = eso2(pathVal[key2]);
if (!key3) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key3];
pathVal = eso3(pathVal[key3]);
if (!key4) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key4];
pathVal = eso4(pathVal[key4]);
return pathVal;
};
}
function getterFnWithEnsureSafeObject(fn, fullExpression) {
return function(s, l) {
return fn(s, l, ensureSafeObject, fullExpression);
};
}
function getterFn(path, options, fullExp) {
var fn = getterFnCache[path];
@@ -919,22 +937,30 @@ function getterFn(path, options, fullExp) {
}
} else {
var code = '';
var needsEnsureSafeObject = false;
forEach(pathKeys, function(key, index) {
ensureSafeMemberName(key, fullExp);
code += 'if(s == null) return undefined;\n' +
's='+ (index
var lookupJs = (index
// we simply dereference 's' on any .dot notation
? 's'
// but if we are first then we check locals first, and if so read it first
: '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '.' + key + ';\n';
: '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '.' + key;
if (isPossiblyDangerousMemberName(key)) {
lookupJs = 'eso(' + lookupJs + ', fe)';
needsEnsureSafeObject = true;
}
code += 'if(s == null) return undefined;\n' +
's=' + lookupJs + ';\n';
});
code += 'return s;';
/* jshint -W054 */
var evaledFnGetter = new Function('s', 'l', code); // s=scope, l=locals
var evaledFnGetter = new Function('s', 'l', 'eso', 'fe', code); // s=scope, l=locals, eso=ensureSafeObject
/* jshint +W054 */
evaledFnGetter.toString = valueFn(code);
if (needsEnsureSafeObject) {
evaledFnGetter = getterFnWithEnsureSafeObject(evaledFnGetter, fullExp);
}
fn = evaledFnGetter;
}

View File

@@ -672,6 +672,24 @@ describe('parser', function() {
describe('sandboxing', function() {
describe('Function constructor', function() {
it('should not tranverse the Function constructor in the getter', function() {
expect(function() {
scope.$eval('{}.toString.constructor');
}).toThrowMinErr(
'$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
'Expression: {}.toString.constructor');
});
it('should not allow access to the Function prototype in the getter', function() {
expect(function() {
scope.$eval('toString.constructor.prototype');
}).toThrowMinErr(
'$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
'Expression: toString.constructor.prototype');
});
it('should NOT allow access to Function constructor in getter', function() {
expect(function() {