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

Commit

Permalink
fix($parse): do not convert to string computed properties multiple times
Browse files Browse the repository at this point in the history
Do not convert to string properties multiple times.
  • Loading branch information
lgalfaso committed Sep 19, 2015
1 parent ded2518 commit 20cf7d5
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 8 deletions.
34 changes: 26 additions & 8 deletions src/ng/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,30 @@ var $parseMinErr = minErr('$parse');


function ensureSafeMemberName(name, fullExpression) {
if (name === "__defineGetter__" || name === "__defineSetter__"
|| name === "__lookupGetter__" || name === "__lookupSetter__"
|| name === "__proto__") {
throw $parseMinErr('isecfld',
'Attempting to access a disallowed field in Angular expressions! '
+ 'Expression: {0}', fullExpression);
}
return name;
}

function getStringValue(name, fullExpression) {
// From the JavaScript docs:
// Property names must be strings. This means that non-string objects cannot be used
// as keys in an object. Any non-string object, including a number, is typecasted
// into a string via the toString method.
//
// So, to ensure that we are checking the same `name` that JavaScript would use,
// we cast it to a string, if possible
name = (isObject(name) && name.toString) ? name.toString() : name;

if (name === "__defineGetter__" || name === "__defineSetter__"
|| name === "__lookupGetter__" || name === "__lookupSetter__"
|| name === "__proto__") {
throw $parseMinErr('isecfld',
'Attempting to access a disallowed field in Angular expressions! '
// we cast it to a string, if possible.
// Doing `name + ''` can cause a repl error if the result to `toString` is not a string,
// this is, this will handle objects that misbehave.
name = name + '';
if (!isString(name)) {
throw $parseMinErr('iseccst',
'Cannot convert object to primitive value! '
+ 'Expression: {0}', fullExpression);
}
return name;
Expand Down Expand Up @@ -816,6 +826,7 @@ ASTCompiler.prototype = {
'ensureSafeMemberName',
'ensureSafeObject',
'ensureSafeFunction',
'getStringValue',
'ifDefined',
'plus',
'text',
Expand All @@ -824,6 +835,7 @@ ASTCompiler.prototype = {
ensureSafeMemberName,
ensureSafeObject,
ensureSafeFunction,
getStringValue,
ifDefined,
plusFn,
expression);
Expand Down Expand Up @@ -967,6 +979,7 @@ ASTCompiler.prototype = {
if (ast.computed) {
right = self.nextId();
self.recurse(ast.property, right);
self.getStringValue(right);
self.addEnsureSafeMemberName(right);
if (create && create !== 1) {
self.if_(self.not(self.computedMember(left, right)), self.lazyAssign(self.computedMember(left, right), '{}'));
Expand Down Expand Up @@ -1187,6 +1200,10 @@ ASTCompiler.prototype = {
return 'ensureSafeFunction(' + item + ',text)';
},

getStringValue: function(item) {
this.assign(item, 'getStringValue(' + item + ',text)');
},

lazyRecurse: function(ast, intoId, nameId, recursionFn, create, skipWatchIdCheck) {
var self = this;
return function() {
Expand Down Expand Up @@ -1561,6 +1578,7 @@ ASTInterpreter.prototype = {
var value;
if (lhs != null) {
rhs = right(scope, locals, assign, inputs);
rhs = getStringValue(rhs);
ensureSafeMemberName(rhs, expression);
if (create && create !== 1 && lhs && !(lhs[rhs])) {
lhs[rhs] = {};
Expand Down
9 changes: 9 additions & 0 deletions test/ng/parseSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2692,6 +2692,15 @@ describe('parser', function() {
});
});

it('should prevent the exploit', function() {
expect(function() {
scope.$eval('(1)[{0: "__proto__", 1: "__proto__", 2: "__proto__", 3: "safe", length: 4, toString: [].pop}].foo = 1');
}).toThrow();
if (!msie || msie > 10) {
expect((1)['__proto__'].foo).toBeUndefined();
}
});

it('should prevent the exploit', function() {
expect(function() {
scope.$eval('' +
Expand Down

0 comments on commit 20cf7d5

Please sign in to comment.