fix($parse): add quick check for Function constructor in fast path

Author: chirayuk

benchmarks/parsed-expressions-bp/app.js

@@ -71,7 +71,8 @@ app.controller('DataController', function($scope, $rootScope) {
       date2: new Date(Math.random()*Date.now()),
       func: function(){ return star; },
       obj: data[i-1],
-      keys: data[i-1] && (data[i-1].keys || Object.keys(data[i-1]))
+      keys: data[i-1] && (data[i-1].keys || Object.keys(data[i-1])),
+      constructor: data[i-1]
     });
   }
 

benchmarks/parsed-expressions-bp/main.html

@@ -16,6 +16,12 @@
           <label for="complexPath">Complex Paths</label>
         </li>
 
+        <li>
+          <input type="radio" ng-model="expressionType" value="constructorPath" id="constructorPath">
+          <label for="constructorPath">Constructor Paths</label>
+          ($parse special cases "constructor" for security)
+        </li>
+
         <li>
           <input type="radio" ng-model="expressionType" value="fieldAccess" id="fieldAccess">
           <label for="fieldAccess">Field Accessors</label>
@@ -77,6 +83,17 @@
           <span bm-pe-watch="row.keys"></span>
         </li>
 
+        <li ng-switch-when="constructorPath" ng-repeat="(rowIdx, row) in ::data">
+          <span bm-pe-watch="row.index"></span>
+          <span bm-pe-watch="row.constructor.index"></span>
+          <span bm-pe-watch="row.constructor.index"></span>
+          <span bm-pe-watch="row.constructor.index"></span>
+          <span bm-pe-watch="row.constructor.constructor.index"></span>
+          <span bm-pe-watch="row.constructor.constructor.index"></span>
+          <span bm-pe-watch="row.constructor.constructor.constructor.index"></span>
+          <span bm-pe-watch="row.constructor.constructor.constructor.index"></span>
+        </li>
+
         <li ng-switch-when="complexPath" ng-repeat="(rowIdx, row) in ::data">
           <span bm-pe-watch="row.index"></span>
           <span bm-pe-watch="row.num0"></span>

src/ng/parse.js

@@ -854,6 +854,10 @@ function setter(obj, path, setValue, fullExp) {
 
 var getterFnCache = createMap();
 
+function isPossiblyDangerousMemberName(name) {
+  return name == 'constructor';
+}
+
 /**
  * Implementation of the "Black Hole" variant from:
  * - http://jsperf.com/angularjs-parse-getter/4
@@ -865,33 +869,47 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) {
   ensureSafeMemberName(key2, fullExp);
   ensureSafeMemberName(key3, fullExp);
   ensureSafeMemberName(key4, fullExp);
+  var eso = function(o) {
+    return ensureSafeObject(o, fullExp);
+  };
+  var eso0 = isPossiblyDangerousMemberName(key0) ? eso : identity;
+  var eso1 = isPossiblyDangerousMemberName(key1) ? eso : identity;
+  var eso2 = isPossiblyDangerousMemberName(key2) ? eso : identity;
+  var eso3 = isPossiblyDangerousMemberName(key3) ? eso : identity;
+  var eso4 = isPossiblyDangerousMemberName(key4) ? eso : identity;
 
   return function cspSafeGetter(scope, locals) {
     var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope;
 
     if (pathVal == null) return pathVal;
-    pathVal = pathVal[key0];
+    pathVal = eso0(pathVal[key0]);
 
     if (!key1) return pathVal;
     if (pathVal == null) return undefined;
-    pathVal = pathVal[key1];
+    pathVal = eso1(pathVal[key1]);
 
     if (!key2) return pathVal;
     if (pathVal == null) return undefined;
-    pathVal = pathVal[key2];
+    pathVal = eso2(pathVal[key2]);
 
     if (!key3) return pathVal;
     if (pathVal == null) return undefined;
-    pathVal = pathVal[key3];
+    pathVal = eso3(pathVal[key3]);
 
     if (!key4) return pathVal;
     if (pathVal == null) return undefined;
-    pathVal = pathVal[key4];
+    pathVal = eso4(pathVal[key4]);
 
     return pathVal;
   };
 }
 
+function getterFnWithEnsureSafeObject(fn, fullExpression) {
+  return function(s, l) {
+    return fn(s, l, ensureSafeObject, fullExpression);
+  };
+}
+
 function getterFn(path, options, fullExp) {
   var fn = getterFnCache[path];
 
@@ -919,22 +937,30 @@ function getterFn(path, options, fullExp) {
     }
   } else {
     var code = '';
+    var needsEnsureSafeObject = false;
     forEach(pathKeys, function(key, index) {
       ensureSafeMemberName(key, fullExp);
-      code += 'if(s == null) return undefined;\n' +
-              's='+ (index
+      var lookupJs = (index
                       // we simply dereference 's' on any .dot notation
                       ? 's'
                       // but if we are first then we check locals first, and if so read it first
-                      : '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '.' + key + ';\n';
+                      : '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '.' + key;
+      if (isPossiblyDangerousMemberName(key)) {
+        lookupJs = 'eso(' + lookupJs + ', fe)';
+        needsEnsureSafeObject = true;
+      }
+      code += 'if(s == null) return undefined;\n' +
+              's=' + lookupJs + ';\n';
     });
     code += 'return s;';
 
     /* jshint -W054 */
-    var evaledFnGetter = new Function('s', 'l', code); // s=scope, l=locals
+    var evaledFnGetter = new Function('s', 'l', 'eso', 'fe', code); // s=scope, l=locals, eso=ensureSafeObject
     /* jshint +W054 */
     evaledFnGetter.toString = valueFn(code);
-
+    if (needsEnsureSafeObject) {
+      evaledFnGetter = getterFnWithEnsureSafeObject(evaledFnGetter, fullExp);
+    }
     fn = evaledFnGetter;
   }
 

test/ng/parseSpec.js

@@ -672,6 +672,24 @@ describe('parser', function() {
 
       describe('sandboxing', function() {
         describe('Function constructor', function() {
+          it('should not tranverse the Function constructor in the getter', function() {
+            expect(function() {
+              scope.$eval('{}.toString.constructor');
+            }).toThrowMinErr(
+                    '$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
+                    'Expression: {}.toString.constructor');
+
+          });
+
+          it('should not allow access to the Function prototype in the getter', function() {
+            expect(function() {
+              scope.$eval('toString.constructor.prototype');
+            }).toThrowMinErr(
+                    '$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
+                    'Expression: toString.constructor.prototype');
+
+          });
+
           it('should NOT allow access to Function constructor in getter', function() {
 
             expect(function() {