diff --git a/src/ng/parse.js b/src/ng/parse.js index 83b126352..2f0d34bbb 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -29,21 +29,31 @@ var promiseWarning; 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! ' - +'Expression: {0}', fullExpression); + // 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; } @@ -722,7 +732,7 @@ Parser.prototype = { return extend(function(self, locals) { var o = obj(self, locals), - i = indexFn(self, locals), + i = getStringValue(indexFn(self, locals), parser.text), v, p; ensureSafeMemberName(i, parser.text); @@ -739,7 +749,7 @@ Parser.prototype = { return v; }, { assign: function(self, value, locals) { - var key = ensureSafeMemberName(indexFn(self, locals), parser.text); + var key = ensureSafeMemberName(getStringValue(indexFn(self, locals), parser.text), parser.text); // prevent overwriting of Function.constructor which would break ensureSafeObject check var o = ensureSafeObject(obj(self, locals), parser.text); if (!o) obj.assign(self, o = {}); diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 6cd2205ea..5c9508641 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -1119,6 +1119,27 @@ describe('parser', function() { expect(count).toBe(1); }); + 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('' + + ' "".sub.call.call(' + + '({})["constructor"].getOwnPropertyDescriptor("".sub.__proto__, "constructor").value,' + + 'null,' + + '"alert(1)"' + + ')()' + + ''); + }).toThrow(); + }); + it('should call the function once when it is part of the context on array lookups', function() { var count = 0;