diff --git a/src/ng/parse.js b/src/ng/parse.js index 700a30d1c191..61a270e7b094 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -753,7 +753,7 @@ Parser.prototype = { var context = contextGetter ? contextGetter(scope, locals) : scope; for (var i = 0; i < argsFn.length; i++) { - args.push(argsFn[i](scope, locals)); + args.push(ensureSafeObject(argsFn[i](scope, locals), parser.text)); } var fnPtr = fn(scope, locals, context) || noop; @@ -841,13 +841,15 @@ Parser.prototype = { ////////////////////////////////////////////////// function setter(obj, path, setValue, fullExp, options) { + ensureSafeObject(obj, fullExp); + //needed? options = options || {}; var element = path.split('.'), key; for (var i = 0; element.length > 1; i++) { key = ensureSafeMemberName(element.shift(), fullExp); - var propertyObj = obj[key]; + var propertyObj = ensureSafeObject(obj[key], fullExp); if (!propertyObj) { propertyObj = {}; obj[key] = propertyObj; @@ -867,7 +869,6 @@ function setter(obj, path, setValue, fullExp, options) { } } key = ensureSafeMemberName(element.shift(), fullExp); - ensureSafeObject(obj, fullExp); ensureSafeObject(obj[key], fullExp); obj[key] = setValue; return setValue; diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 6a72f11d3527..6e827d98a1f3 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -1006,6 +1006,43 @@ describe('parser', function() { expect(scope.$eval('fn().anotherFn()')).toBe(true); }); + it('should disallow traversing the Function object in a setter: E02', function() { + expect(function() { + // This expression by itself isn't dangerous. However, one can use this to + // automatically call an object (e.g. a Function object) when it is automatically + // toString'd/valueOf'd by setting the RHS to Function.prototype.call. + scope.$eval('hasOwnProperty.constructor.prototype.valueOf = 1'); + }).toThrowMinErr( + '$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' + + 'Expression: hasOwnProperty.constructor.prototype.valueOf = 1'); + }); + + it('should disallow passing the Function object as a parameter: E03', function() { + expect(function() { + // This expression constructs a function but does not execute it. It does lead the + // way to execute it if one can get the toString/valueOf of it to call the function. + scope.$eval('["a", "alert(1)"].sort(hasOwnProperty.constructor)'); + }).toThrow(); + }); + + it('should prevent exploit E01', function() { + // This is a tracking exploit. The two individual tests, it('should … : E02') and + // it('should … : E03') test for two parts to block this exploit. This exploit works + // as follows: + // + // • Array.sort takes a comparison function and passes it 2 parameters to compare. If + // the result is non-primitive, sort then invokes valueOf() on the result. + // • The Function object conveniently accepts two string arguments so we can use this + // to construct a function. However, this doesn't do much unless we can execute it. + // • We set the valueOf property on Function.prototype to Function.prototype.call. + // This causes the function that we constructed to be executed when sort calls + // .valueOf() on the result of the comparison. + expect(function() { + scope.$eval('' + + 'hasOwnProperty.constructor.prototype.valueOf=valueOf.call;' + + '["a","alert(1)"].sort(hasOwnProperty.constructor)'); + }).toThrow(); + }); it('should call the function once when it is part of the context', function() { var count = 0;