feat($parse): secure expressions by hiding "private" properties

BREAKING CHANGE:
This commit introduces the notion of "private" properties (properties
whose names begin and/or end with an underscore) on the scope chain.
These properties will not be available to Angular expressions (i.e. {{
}} interpolation in templates and strings passed to `$parse`)  They are
freely available to JavaScript code (as before).

Motivation
----------
Angular expressions execute in a limited context.  They do not have
direct access to the global scope, Window, Document or the Function
constructor.  However, they have direct access to names/properties on
the scope chain.  It has been a long standing best practice to keep
sensitive APIs outside of the scope chain (in a closure or your
controller.)  That's easier said that done for two reasons: (1)
JavaScript does not have a notion of private properties so if you need
someone on the scope chain for JavaScript use, you also expose it to
Angular expressions, and (2) the new "controller as" syntax that's now
in increased usage exposes the entire controller on the scope chain
greatly increaing the exposed surface.  Though Angular expressions are
written and controlled by the developer, they (1) typically deal with
user input and (2) don't get the kind of test coverage that JavaScript
code would.  This commit provides a way, via a naming convention, to
allow publishing/restricting properties from controllers/scopes to
Angular expressions enabling one to only expose those properties that
are actually needed by the expressions.

1 files changed, 25 insertions, 9 deletions

diff --git a/src/ng/parse.js b/src/ng/parse.js
index c93d07de..ede3f24b 100644
--- a/src/ng/parse.js
+++ b/src/ng/parse.js
@@ -8,18 +8,23 @@ var promiseWarning;
 // ------------------------------
 // Angular expressions are generally considered safe because these expressions only have direct
 // access to $scope and locals. However, one can obtain the ability to execute arbitrary JS code by
-// obtaining a reference to native JS functions such as the Function constructor.
+// obtaining a reference to native JS functions such as the Function constructor, thw global Window
+// or Document object.  In addition, many powerful functions for use by JavaScript code are
+// published on scope that shouldn't be available from within an Angular expression.
 //
 // As an example, consider the following Angular expression:
 //
 //   {}.toString.constructor(alert("evil JS code"))
 //
 // We want to prevent this type of access. For the sake of performance, during the lexing phase we
-// disallow any "dotted" access to any member named "constructor".
+// disallow any "dotted" access to any member named "constructor" or to any member whose name begins
+// or ends with an underscore.  The latter allows one to exclude the private / JavaScript only API
+// available on the scope and controllers from the context of an Angular expression.
 //
-// For reflective calls (a[b]) we check that the value of the lookup is not the Function constructor
-// while evaluating the expression, which is a stronger but more expensive test. Since reflective
-// calls are expensive anyway, this is not such a big deal compared to static dereferencing.
+// For reflective calls (a[b]), we check that the value of the lookup is not the Function
+// constructor, Window or DOM node while evaluating the expression, which is a stronger but more
+// expensive test. Since reflective calls are expensive anyway, this is not such a big deal compared
+// to static dereferencing.
 //
 // This sandboxing technique is not perfect and doesn't aim to be. The goal is to prevent exploits
 // against the expression language, but not to prevent exploits that were enabled by exposing
@@ -33,12 +38,20 @@ var promiseWarning;
 // In general, it is not possible to access a Window object from an angular expression unless a
 // window or some DOM object that has a reference to window is published onto a Scope.
 
-function ensureSafeMemberName(name, fullExpression) {
-  if (name === "constructor") {
+function ensureSafeMemberName(name, fullExpression, allowConstructor) {
+  if (typeof name !== 'string' && toString.apply(name) !== "[object String]") {
+    return name;
+  }
+  if (name === "constructor" && !allowConstructor) {
     throw $parseMinErr('isecfld',
         'Referencing "constructor" field in Angular expressions is disallowed! Expression: {0}',
         fullExpression);
   }
+  if (name.charAt(0) === '_' || name.charAt(name.length-1) === '_') {
+    throw $parseMinErr('isecprv',
+        'Referencing private fields in Angular expressions is disallowed! Expression: {0}',
+        fullExpression);
+  }
   return name;
 }
 
@@ -722,7 +735,10 @@ Parser.prototype = {
 
     return extend(function(self, locals) {
       var o = obj(self, locals),
-          i = indexFn(self, locals),
+          // In the getter, we will not block looking up "constructor" by name in order to support user defined
+          // constructors.  However, if value looked up is the Function constructor, we will still block it in the
+          // ensureSafeObject call right after we look up o[i] (a few lines below.)
+          i = ensureSafeMemberName(indexFn(self, locals), parser.text, true /* allowConstructor */),
           v, p;
 
       if (!o) return undefined;
@@ -738,7 +754,7 @@ Parser.prototype = {
       return v;
     }, {
       assign: function(self, value, locals) {
-        var key = indexFn(self, locals);
+        var key = ensureSafeMemberName(indexFn(self, locals), parser.text);
         // prevent overwriting of Function.constructor which would break ensureSafeObject check
         var safe = ensureSafeObject(obj(self, locals), parser.text);
         return safe[key] = value;