Prevent access to other prototype properties

lib/handlebars/compiler/compiler.js

@@ -3,6 +3,7 @@
 import Exception from '../exception';
 import {isArray, indexOf, extend} from '../utils';
 import AST from './ast';
+import {newObjectWithoutPrototypeProperties} from '../internal/newObjectWithoutPrototypeProperties';
 
 const slice = [].slice;
 
@@ -56,7 +57,8 @@ Compiler.prototype = {
 
     // These changes will propagate to the other compiler components
     let knownHelpers = options.knownHelpers;
-    options.knownHelpers = {
+
+    options.knownHelpers = newObjectWithoutPrototypeProperties({
       'helperMissing': true,
       'blockHelperMissing': true,
       'each': true,
@@ -65,14 +67,11 @@ Compiler.prototype = {
       'with': true,
       'log': true,
       'lookup': true
-    };
+    });
     if (knownHelpers) {
-      // the next line should use "Object.keys", but the code has been like this a long time and changing it, might
-      // cause backwards-compatibility issues... It's an old library...
-      // eslint-disable-next-line guard-for-in
-      for (let name in knownHelpers) {
-          this.options.knownHelpers[name] = knownHelpers[name];
-      }
+      Object.keys(knownHelpers).forEach(name => {
+        this.options.knownHelpers[name] = knownHelpers[name];
+      });
     }
 
     return this.accept(program);

lib/handlebars/compiler/javascript-compiler.js

@@ -2,6 +2,7 @@ import { COMPILER_REVISION, REVISION_CHANGES } from '../base';
 import Exception from '../exception';
 import {isArray} from '../utils';
 import CodeGen from './code-gen';
+import {propertyMustBeEnumerable} from '../internal/propertyMustBeEnumerable';
 
 function Literal(value) {
   this.value = value;
@@ -13,9 +14,9 @@ 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*/) {
-    const isEnumerable = [ this.aliasable('container.propertyIsEnumerable'), '.call(', parent, ',"constructor")'];
+    const isEnumerable = [ this.aliasable('container.propertyIsEnumerable'), '.call(', parent, ',', JSON.stringify(name), ')'];
 
-    if (name === 'constructor') {
+    if (propertyMustBeEnumerable(name)) {
       return ['(', isEnumerable, '?', _actualLookup(), ' : undefined)'];
     }
     return _actualLookup();
@@ -93,6 +94,7 @@ JavaScriptCompiler.prototype = {
 
     this.useDepths = this.useDepths || environment.useDepths || environment.useDecorators || this.options.compat;
     this.useBlockParams = this.useBlockParams || environment.useBlockParams;
+    this.allowNonHelperFunctionCall = this.options.allowNonHelperFunctionCall !== false; // default if true for 4.x
 
     let opcodes = environment.opcodes,
         opcode,
@@ -634,12 +636,20 @@ JavaScriptCompiler.prototype = {
     if (isSimple) { // direct call to helper
       possibleFunctionCalls.push(helper.name);
     }
+
     // call a function from the input object
-    possibleFunctionCalls.push(nonHelper);
+    if (this.allowNonHelperFunctionCall) {
+      possibleFunctionCalls.push(nonHelper);
+    }
+
     if (!this.options.strict) {
       possibleFunctionCalls.push(this.aliasable('container.hooks.helperMissing'));
     }
 
+    if (possibleFunctionCalls.length === 0) {
+      throw new Exception('Cannot create code for calling "' + name + '": Non-helper function calls are not allowed.');
+    }
+
     let functionLookupCode = ['(', this.itemsSeparatedBy(possibleFunctionCalls, '||'), ')'];
     let functionCall = this.source.functionCall(functionLookupCode, 'call', helper.callParams);
     this.push(functionCall);
@@ -1009,6 +1019,7 @@ JavaScriptCompiler.prototype = {
     let params = [],
         paramsInit = this.setupHelperArgs(name, paramSize, params, blockHelper);
     let foundHelper = this.nameLookup('helpers', name, 'helper'),
+        // NEXT-MAJOR-UPGRADE: Always use "nullContext" in Handlebars 5.0
         callContext = this.aliasable(`${this.contextName(0)} != null ? ${this.contextName(0)} : (container.nullContext || {})`);
 
     return {

lib/handlebars/helpers/lookup.js

@@ -1,9 +1,11 @@
+import {propertyMustBeEnumerable} from '../internal/propertyMustBeEnumerable';
+
 export default function(instance) {
   instance.registerHelper('lookup', function(obj, field) {
     if (!obj) {
       return obj;
     }
-    if (String(field) === 'constructor' && !obj.propertyIsEnumerable(field)) {
+    if (propertyMustBeEnumerable(field) && !obj.propertyIsEnumerable(field)) {
       return undefined;
     }
     return obj[field];

lib/handlebars/internal/newObjectWithoutPrototypeProperties.js

@@ -0,0 +1,23 @@
+/**
+ * Create an object without Object.prototype methods like __defineGetter__, constructor,
+ * __defineSetter__ and __proto__.
+ *
+ * Those methods should not be accessed from template code, because that can lead to
+ * security leaks. This method should be used to create internal objects that.
+ *
+ * @private
+ * @param {...object} sourceObjects
+ * @returns {object}
+ */
+export function newObjectWithoutPrototypeProperties(...sourceObjects) {
+  let result = Object.create(null);
+  sourceObjects.forEach(sourceObject => {
+    if (sourceObject != null) {
+      Object.keys(sourceObject).forEach(key => {
+        result[key] = sourceObject[key];
+      });
+    }
+  });
+  return result;
+}
+

lib/handlebars/internal/propertyMustBeEnumerable.js

@@ -0,0 +1,59 @@
+import {newObjectWithoutPrototypeProperties} from './newObjectWithoutPrototypeProperties';
+
+const dangerousProperties = newObjectWithoutPrototypeProperties();
+
+getFunctionPropertiesOf(Object.prototype).forEach(propertyName => {
+  dangerousProperties[propertyName] = true;
+});
+getFunctionPropertiesOf(Array.prototype).forEach(propertyName => {
+  dangerousProperties[propertyName] = true;
+});
+getFunctionPropertiesOf(Function.prototype).forEach(propertyName => {
+  dangerousProperties[propertyName] = true;
+});
+getFunctionPropertiesOf(String.prototype).forEach(propertyName => {
+  dangerousProperties[propertyName] = true;
+});
+
+// eslint-disable-next-line no-proto
+dangerousProperties.__proto__ = true;
+dangerousProperties.__defineGetter__ = true;
+dangerousProperties.__defineSetter__ = true;
+
+// Following properties are not _that_ dangerous
+delete dangerousProperties.toString;
+delete dangerousProperties.includes;
+delete dangerousProperties.slice;
+delete dangerousProperties.isPrototypeOf;
+delete dangerousProperties.propertyIsEnumerable;
+delete dangerousProperties.indexOf;
+delete dangerousProperties.keys;
+delete dangerousProperties.lastIndexOf;
+
+function getFunctionPropertiesOf(obj) {
+  return Object.getOwnPropertyNames(obj)
+      .filter(propertyName => {
+        try {
+          return typeof obj[propertyName] === 'function';
+        } catch (error) {
+          // TypeError happens here when accessing 'caller', 'callee', 'arguments' on Function
+          return true;
+        }
+      });
+}
+
+/**
+ * Checks if a property can be harmful and should only processed when it is enumerable on its parent.
+ *
+ * This is necessary because of various "arbitrary-code-execution" issues that Handlebars has faced in the past.
+ *
+ * @param propertyName
+ * @returns {boolean}
+ */
+export function propertyMustBeEnumerable(propertyName) {
+  return dangerousProperties[propertyName];
+}
+
+export function getAllDangerousProperties() {
+  return dangerousProperties;
+}

lib/handlebars/runtime.js

@@ -2,6 +2,7 @@ import * as Utils from './utils';
 import Exception from './exception';
 import {COMPILER_REVISION, createFrame, LAST_COMPATIBLE_COMPILER_REVISION, REVISION_CHANGES} from './base';
 import {moveHelperToHooks} from './helpers';
+import {newObjectWithoutPrototypeProperties} from './internal/newObjectWithoutPrototypeProperties';
 
 export function checkRevision(compilerInfo) {
   const compilerRevision = compilerInfo && compilerInfo[0] || 1,
@@ -158,13 +159,13 @@ export function template(templateSpec, env) {
 
   ret._setup = function(options) {
     if (!options.partial) {
-      container.helpers = Utils.extend({}, env.helpers, options.helpers);
+      container.helpers = newObjectWithoutPrototypeProperties(env.helpers, options.helpers);
 
       if (templateSpec.usePartial) {
-        container.partials = Utils.extend({}, env.partials, options.partials);
+        container.partials = newObjectWithoutPrototypeProperties(env.partials, options.partials);
       }
       if (templateSpec.usePartial || templateSpec.useDecorators) {
-        container.decorators = Utils.extend({}, env.decorators, options.decorators);
+        container.decorators = newObjectWithoutPrototypeProperties(env.decorators, options.decorators);
       }
 
       container.hooks = {};

spec/security.js

@@ -113,14 +113,67 @@ describe('security issues', function() {
 
     describe('GH-1563', function() {
         it('should not allow to access constructor after overriding via __defineGetter__', function() {
-            if (({}).__defineGetter__ == null || ({}).__lookupGetter__ == null) {
-                return; // Browser does not support this exploit anyway
-            }
-            shouldCompileTo('{{__defineGetter__ "undefined" valueOf }}' +
-                '{{#with __lookupGetter__ }}' +
-                '{{__defineGetter__ "propertyIsEnumerable" (this.bind (this.bind 1)) }}' +
-                '{{constructor.name}}' +
-                '{{/with}}', {}, '');
+
+            shouldThrow(function() {
+                compileWithPartials('{{__defineGetter__ "undefined" valueOf }}' +
+                    '{{#with __lookupGetter__ }}' +
+                    '{{__defineGetter__ "propertyIsEnumerable" (this.bind (this.bind 1)) }}' +
+                    '{{constructor.name}}' +
+                    '{{/with}}', [{}, {}, {}, {}])({});
+                }
+            );
+        });
+    });
+
+    describe('the compile option "allowNonHelperFunctionCall"', function() {
+        it('when set to false should prevent calling functions in input objects', function() {
+            shouldThrow(function() {
+                var template = compileWithPartials('{{test abc}}', [{}, {}, {}, {allowNonHelperFunctionCall: false}]);
+                template({test: function() { return 'abc'; }});
+            }, null, /Missing helper/);
+        });
+        it('when set to false should prevent calling functions in input objects (in strict mode)', function() {
+            shouldThrow(function() {
+                var template = compileWithPartials('{{obj.method abc}}', [{}, {}, {}, {allowNonHelperFunctionCall: false, strict: true}]);
+                template({});
+            }, null, /Cannot create code.*obj\.method.*Non-helper/);
+        });
+
+    });
+
+    describe('Properties that are required to be enumerable', function() {
+
+        it('access should be restricted if not enumerable', function() {
+            shouldCompileTo('{{__defineGetter__}}', {}, '');
+            shouldCompileTo('{{__defineSetter__}}', {}, '');
+            shouldCompileTo('{{__proto__}}', {}, '');
+            shouldCompileTo('{{constructor}}', {}, '');
+        });
+
+        it('access should be allowed if enumerable', function() {
+            shouldCompileTo('{{__defineGetter__}}', {__defineGetter__: 'abc'}, 'abc');
+            shouldCompileTo('{{__defineSetter__}}', {__defineSetter__: 'abc'}, 'abc');
+            shouldCompileTo('{{constructor}}', {constructor: 'abc'}, 'abc');
+        });
+
+        it('access can be allowed via the compile-option "propertyMustBeEnumerable"', function() {
+            var context = {};
+            ['__defineGetter__', '__defineSetter__'].forEach(function(property) {
+                Object.defineProperty(context, property, {
+                    get: function() {
+                        return property;
+                    },
+                    enumerable: false
+                });
+            });
+
+            var compileOptions = {
+                propertyMustBeEnumerable: {
+                    __defineGetter__: false
+                }
+            };
+
+            shouldCompileTo('{{__defineGetter__}}{{__defineSetter__}}', [context, {}, {}, compileOptions], '__defineGetter__');
         });
     });
 });

types/index.d.ts

@@ -214,6 +214,7 @@ interface CompileOptions {
   preventIndent?: boolean;
   ignoreStandalone?: boolean;
   explicitPartialContext?: boolean;
+  allowNonHelperFunctionCall?: boolean;
 }
 
 type KnownHelpers = {