feat: access control to prototype properties via whitelist

lib/handlebars/compiler/javascript-compiler.js

@@ -2,7 +2,6 @@ import { COMPILER_REVISION, REVISION_CHANGES } from '../base';
 import Exception from '../exception';
 import { isArray } from '../utils';
 import CodeGen from './code-gen';
-import { dangerousPropertyRegex } from '../helpers/lookup';
 
 function Literal(value) {
   this.value = value;
@@ -13,27 +12,8 @@ function JavaScriptCompiler() {}
 JavaScriptCompiler.prototype = {
   // PUBLIC API: You can override these methods in a subclass to provide
   // alternative compiled forms for name lookup and buffering semantics
-  nameLookup: function(parent, name /* , type*/) {
-    if (dangerousPropertyRegex.test(name)) {
-      const isEnumerable = [
-        this.aliasable('container.propertyIsEnumerable'),
-        '.call(',
-        parent,
-        ',',
-        JSON.stringify(name),
-        ')'
-      ];
-      return ['(', isEnumerable, '?', _actualLookup(), ' : undefined)'];
-    }
-    return _actualLookup();
-
-    function _actualLookup() {
-      if (JavaScriptCompiler.isValidJavaScriptVariableName(name)) {
-        return [parent, '.', name];
-      } else {
-        return [parent, '[', JSON.stringify(name), ']'];
-      }
-    }
+  nameLookup: function(parent, name /*,  type */) {
+    return this.internalNameLookup(parent, name);
   },
   depthedLookup: function(name) {
     return [this.aliasable('container.lookup'), '(depths, "', name, '")'];
@@ -69,6 +49,12 @@ JavaScriptCompiler.prototype = {
     return this.quotedString('');
   },
   // END PUBLIC API
+  internalNameLookup: function(parent, name) {
+    this.lookupPropertyFunctionIsUsed = true;
+    return ['lookupProperty(', parent, ',', JSON.stringify(name), ')'];
+  },
+
+  lookupPropertyFunctionIsUsed: false,
 
   compile: function(environment, options, context, asObject) {
     this.environment = environment;
@@ -131,7 +117,11 @@ JavaScriptCompiler.prototype = {
     if (!this.decorators.isEmpty()) {
       this.useDecorators = true;
 
-      this.decorators.prepend('var decorators = container.decorators;\n');
+      this.decorators.prepend([
+        'var decorators = container.decorators, ',
+        this.lookupPropertyFunctionVarDeclaration(),
+        ';\n'
+      ]);
       this.decorators.push('return fn;');
 
       if (asObject) {
@@ -248,6 +238,10 @@ JavaScriptCompiler.prototype = {
       }
     });
 
+    if (this.lookupPropertyFunctionIsUsed) {
+      varDeclarations += ', ' + this.lookupPropertyFunctionVarDeclaration();
+    }
+
     let params = ['container', 'depth0', 'helpers', 'partials', 'data'];
 
     if (this.useBlockParams || this.useDepths) {
@@ -335,6 +329,17 @@ JavaScriptCompiler.prototype = {
     return this.source.merge();
   },
 
+  lookupPropertyFunctionVarDeclaration: function() {
+    return `
+      lookupProperty = container.lookupProperty || function(parent, propertyName) {
+        if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
+          return parent[propertyName];
+        }
+        return undefined
+    }
+    `.trim();
+  },
+
   // [blockValue]
   //
   // On stack, before: hash, inverse, program, value
@@ -1241,6 +1246,9 @@ JavaScriptCompiler.prototype = {
   }
 })();
 
+/**
+ * @deprecated May be removed in the next major version
+ */
 JavaScriptCompiler.isValidJavaScriptVariableName = function(name) {
   return (
     !JavaScriptCompiler.RESERVED_WORDS[name] &&

lib/handlebars/helpers/lookup.js

@@ -1,16 +1,9 @@
-export const dangerousPropertyRegex = /^(constructor|__defineGetter__|__defineSetter__|__lookupGetter__|__proto__)$/;
-
 export default function(instance) {
-  instance.registerHelper('lookup', function(obj, field) {
+  instance.registerHelper('lookup', function(obj, field, options) {
     if (!obj) {
+      // Note for 5.0: Change to "obj == null" in 5.0
       return obj;
     }
-    if (
-      dangerousPropertyRegex.test(String(field)) &&
-      !Object.prototype.propertyIsEnumerable.call(obj, field)
-    ) {
-      return undefined;
-    }
-    return obj[field];
+    return options.lookupProperty(obj, field);
   });
 }

lib/handlebars/internal/createNewLookupObject.js

@@ -0,0 +1,11 @@
+import { extend } from '../utils';
+
+/**
+ * Create a new object with "null"-prototype to avoid truthy results on prototype properties.
+ * The resulting object can be used with "object[property]" to check if a property exists
+ * @param {...object} sources a varargs parameter of source objects that will be merged
+ * @returns {object}
+ */
+export function createNewLookupObject(...sources) {
+  return extend(Object.create(null), ...sources);
+}

lib/handlebars/internal/wrapHelper.js

@@ -0,0 +1,8 @@
+export function wrapHelper(helper, transformOptionsFn) {
+  let wrapper = function(/* dynamic arguments */) {
+    const options = arguments[arguments.length - 1];
+    arguments[arguments.length - 1] = transformOptionsFn(options);
+    return helper.apply(this, arguments);
+  };
+  return wrapper;
+}

lib/handlebars/runtime.js

@@ -7,6 +7,8 @@ import {
   REVISION_CHANGES
 } from './base';
 import { moveHelperToHooks } from './helpers';
+import { wrapHelper } from './internal/wrapHelper';
+import { createNewLookupObject } from './internal/createNewLookupObject';
 
 export function checkRevision(compilerInfo) {
   const compilerRevision = (compilerInfo && compilerInfo[0]) || 1,
@@ -69,13 +71,17 @@ export function template(templateSpec, env) {
     }
     partial = env.VM.resolvePartial.call(this, partial, context, options);
 
-    let optionsWithHooks = Utils.extend({}, options, { hooks: this.hooks });
+    let extendedOptions = Utils.extend({}, options, {
+      hooks: this.hooks,
+      allowedProtoMethods: this.allowedProtoMethods,
+      allowedProtoProperties: this.allowedProtoProperties
+    });
 
     let result = env.VM.invokePartial.call(
       this,
       partial,
       context,
-      optionsWithHooks
+      extendedOptions
     );
 
     if (result == null && env.compile) {
@@ -84,7 +90,7 @@ export function template(templateSpec, env) {
         templateSpec.compilerOptions,
         env
       );
-      result = options.partials[options.name](context, optionsWithHooks);
+      result = options.partials[options.name](context, extendedOptions);
     }
     if (result != null) {
       if (options.indent) {
@@ -118,10 +124,26 @@ export function template(templateSpec, env) {
       }
       return obj[name];
     },
+    lookupProperty: function(parent, propertyName) {
+      let result = parent[propertyName];
+      if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
+        return result;
+      }
+      const whitelist =
+        typeof result === 'function'
+          ? container.allowedProtoMethods
+          : container.allowedProtoProperties;
+
+      if (whitelist[propertyName] === true) {
+        return result;
+      }
+      return undefined;
+    },
     lookup: function(depths, name) {
       const len = depths.length;
       for (let i = 0; i < len; i++) {
-        if (depths[i] && depths[i][name] != null) {
+        let result = depths[i] && container.lookupProperty(depths[i], name);
+        if (result != null) {
           return depths[i][name];
         }
       }
@@ -229,7 +251,9 @@ export function template(templateSpec, env) {
 
   ret._setup = function(options) {
     if (!options.partial) {
-      container.helpers = Utils.extend({}, env.helpers, options.helpers);
+      let mergedHelpers = Utils.extend({}, env.helpers, options.helpers);
+      wrapHelpersToPassLookupProperty(mergedHelpers, container);
+      container.helpers = mergedHelpers;
 
       if (templateSpec.usePartial) {
         // Use mergeIfNeeded here to prevent compiling global partials multiple times
@@ -247,13 +271,21 @@ export function template(templateSpec, env) {
       }
 
       container.hooks = {};
+      container.allowedProtoProperties = createNewLookupObject(
+        options.allowedProtoProperties
+      );
+      container.allowedProtoMethods = createNewLookupObject(
+        options.allowedProtoMethods
+      );
 
       let keepHelperInHelpers =
         options.allowCallsToHelperMissing ||
         templateWasPrecompiledWithCompilerV7;
       moveHelperToHooks(container, 'helperMissing', keepHelperInHelpers);
       moveHelperToHooks(container, 'blockHelperMissing', keepHelperInHelpers);
     } else {
+      container.allowedProtoProperties = options.allowedProtoProperties;
+      container.allowedProtoMethods = options.allowedProtoMethods;
       container.helpers = options.helpers;
       container.partials = options.partials;
       container.decorators = options.decorators;
@@ -405,3 +437,17 @@ function executeDecorators(fn, prog, container, depths, data, blockParams) {
   }
   return prog;
 }
+
+function wrapHelpersToPassLookupProperty(mergedHelpers, container) {
+  Object.keys(mergedHelpers).forEach(helperName => {
+    let helper = mergedHelpers[helperName];
+    mergedHelpers[helperName] = passLookupPropertyOption(helper, container);
+  });
+}
+
+function passLookupPropertyOption(helper, container) {
+  const lookupProperty = container.lookupProperty;
+  return wrapHelper(helper, options => {
+    return Utils.extend({ lookupProperty }, options);
+  });
+}

package.json

@@ -35,6 +35,7 @@
     "babel-runtime": "^5.1.10",
     "benchmark": "~1.0",
     "chai": "^4.2.0",
+    "chai-diff": "^1.0.1",
     "concurrently": "^5.0.0",
     "dirty-chai": "^2.0.1",
     "dtslint": "^0.5.5",
@@ -84,7 +85,7 @@
   "scripts": {
     "format": "prettier --write '**/*.js' && eslint --fix .",
     "check-format": "prettier --check '**/*.js'",
-    "lint": "eslint --max-warnings 0 . ",
+    "lint": "eslint --max-warnings 0 .",
     "dtslint": "dtslint types",
     "test": "grunt",
     "extensive-tests-and-publish-to-aws": "npx mocha tasks/task-tests/ && grunt --stack extensive-tests-and-publish-to-aws",

spec/blocks.js

@@ -275,6 +275,7 @@ describe('blocks', function() {
         'Goodbye cruel OMG!'
       );
     });
+
     it('block with deep recursive pathed lookup', function() {
       var string =
         '{{#outer}}Goodbye {{#inner}}cruel {{omg.yes}}{{/inner}}{{/outer}}';

spec/env/common.js

@@ -167,7 +167,7 @@ HandlebarsTestBench.prototype.withMessage = function(message) {
 };
 
 HandlebarsTestBench.prototype.toCompileTo = function(expectedOutputAsString) {
-  expect(this._compileAndExeute()).to.equal(expectedOutputAsString);
+  expect(this._compileAndExecute()).to.equal(expectedOutputAsString);
 };
 
 // see chai "to.throw" (https://www.chaijs.com/api/bdd/#method_throw)
@@ -178,11 +178,11 @@ HandlebarsTestBench.prototype.toThrow = function(
 ) {
   var self = this;
   expect(function() {
-    self._compileAndExeute();
+    self._compileAndExecute();
   }).to.throw(errorLike, errMsgMatcher, msg);
 };
 
-HandlebarsTestBench.prototype._compileAndExeute = function() {
+HandlebarsTestBench.prototype._compileAndExecute = function() {
   var compile =
     Object.keys(this.partials).length > 0
       ? CompilerContext.compileWithPartial

spec/helpers.js

@@ -1328,4 +1328,15 @@ describe('helpers', function() {
       );
     });
   });
+
+  describe('the lookupProperty-option', function() {
+    it('should be passed to custom helpers', function() {
+      expectTemplate('{{testHelper}}')
+        .withHelper('testHelper', function testHelper(options) {
+          return options.lookupProperty(this, 'testProperty');
+        })
+        .withInput({ testProperty: 'abc' })
+        .toCompileTo('abc');
+    });
+  });
 });

spec/javascript-compiler.js

@@ -81,4 +81,29 @@ describe('javascript-compiler api', function() {
       shouldCompileTo('{{foo}}', { foo: 'food' }, 'food_foo');
     });
   });
+
+  describe('#isValidJavaScriptVariableName', function() {
+    // It is there and accessible and could be used by someone. That's why we don't remove it
+    // it 4.x. But if we keep it, we add a test
+    // This test should not encourage you to use the function. It is not needed any more
+    // and might be removed in 5.0
+    ['test', 'abc123', 'abc_123'].forEach(function(validVariableName) {
+      it("should return true for '" + validVariableName + "'", function() {
+        expect(
+          handlebarsEnv.JavaScriptCompiler.isValidJavaScriptVariableName(
+            validVariableName
+          )
+        ).to.be.true();
+      });
+    });
+    [('123test', 'abc()', 'abc.cde')].forEach(function(invalidVariableName) {
+      it("should return true for '" + invalidVariableName + "'", function() {
+        expect(
+          handlebarsEnv.JavaScriptCompiler.isValidJavaScriptVariableName(
+            invalidVariableName
+          )
+        ).to.be.false();
+      });
+    });
+  });
 });