Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix($parse): disallow passing Function to Array.sort
Browse files Browse the repository at this point in the history
Fix the following exploit:

    hasOwnProperty.constructor.prototype.valueOf = valueOf.call;
    ["a", "alert(1)"].sort(hasOwnProperty.constructor);

The exploit:
• 1. Array.sort takes a comparison function and passes it 2 parameters to compare.
  2. It then calls .valueOf() if the result is not a primitive.
• 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 function 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.

The fix is in two parts.
• Disallow passing unsafe objects to function calls as parameters.
• Do not traverse the Function object when setting a path.
  • Loading branch information
chirayuk authored and btford committed Sep 9, 2014
1 parent 5061d2c commit b39e1d4
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/ng/parse.js
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
37 changes: 37 additions & 0 deletions test/ng/parseSpec.js
Expand Up @@ -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;
Expand Down

0 comments on commit b39e1d4

Please sign in to comment.