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

Commit d434f3d

Browse files
committed
fix($parse): do not convert to string computed properties multiple times
Do not convert to string properties multiple times.
1 parent 7e08975 commit d434f3d

File tree

2 files changed

+29
-10
lines changed

2 files changed

+29
-10
lines changed

src/ng/parse.js

+20-10
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,30 @@ var $parseMinErr = minErr('$parse');
3838

3939

4040
function ensureSafeMemberName(name, fullExpression) {
41+
if (name === "__defineGetter__" || name === "__defineSetter__"
42+
|| name === "__lookupGetter__" || name === "__lookupSetter__"
43+
|| name === "__proto__") {
44+
throw $parseMinErr('isecfld',
45+
'Attempting to access a disallowed field in Angular expressions! '
46+
+ 'Expression: {0}', fullExpression);
47+
}
48+
return name;
49+
}
50+
51+
function getStringValue(name, fullExpression) {
4152
// From the JavaScript docs:
4253
// Property names must be strings. This means that non-string objects cannot be used
4354
// as keys in an object. Any non-string object, including a number, is typecasted
4455
// into a string via the toString method.
4556
//
4657
// So, to ensure that we are checking the same `name` that JavaScript would use,
47-
// we cast it to a string, if possible
48-
name = (isObject(name) && name.toString) ? name.toString() : name;
49-
50-
if (name === "__defineGetter__" || name === "__defineSetter__"
51-
|| name === "__lookupGetter__" || name === "__lookupSetter__"
52-
|| name === "__proto__") {
53-
throw $parseMinErr('isecfld',
54-
'Attempting to access a disallowed field in Angular expressions! '
58+
// we cast it to a string, if possible.
59+
// Doing `name + ''` can cause a repl error if the result to `toString` is not a string,
60+
// this is, this will handle objects that misbehave.
61+
name = name + '';
62+
if (!isString(name)) {
63+
throw $parseMinErr('iseccst',
64+
'Cannot convert object to primitive value! '
5565
+ 'Expression: {0}', fullExpression);
5666
}
5767
return name;
@@ -698,7 +708,7 @@ Parser.prototype = {
698708

699709
return extend(function $parseObjectIndex(self, locals) {
700710
var o = obj(self, locals),
701-
i = indexFn(self, locals),
711+
i = getStringValue(indexFn(self, locals), expression),
702712
v;
703713

704714
ensureSafeMemberName(i, expression);
@@ -707,7 +717,7 @@ Parser.prototype = {
707717
return v;
708718
}, {
709719
assign: function(self, value, locals) {
710-
var key = ensureSafeMemberName(indexFn(self, locals), expression);
720+
var key = ensureSafeMemberName(getStringValue(indexFn(self, locals), expression), expression);
711721
// prevent overwriting of Function.constructor which would break ensureSafeObject check
712722
var o = ensureSafeObject(obj(self, locals), expression);
713723
if (!o) obj.assign(self, o = {}, locals);

test/ng/parseSpec.js

+9
Original file line numberDiff line numberDiff line change
@@ -1215,6 +1215,15 @@ describe('parser', function() {
12151215
});
12161216
});
12171217

1218+
it('should prevent the exploit', function() {
1219+
expect(function() {
1220+
scope.$eval('(1)[{0: "__proto__", 1: "__proto__", 2: "__proto__", 3: "safe", length: 4, toString: [].pop}].foo = 1');
1221+
}).toThrow();
1222+
if (!msie || msie > 10) {
1223+
expect((1)['__proto__'].foo).toBeUndefined();
1224+
}
1225+
});
1226+
12181227
it('should prevent the exploit', function() {
12191228
expect(function() {
12201229
scope.$eval('' +

0 commit comments

Comments
 (0)