fixup: "lookup" should also respect dangerous properties

- properties not configurable anymore
- add more harmful properties to the list
  (push, pop, splice...)

lib/handlebars/compiler/compiler.js

@@ -3,7 +3,7 @@
 import Exception from '../exception';
 import {isArray, indexOf, extend} from '../utils';
 import AST from './ast';
-import {newMapObject} from '../internal/utils';
+import {newObjectWithoutPrototypeProperties} from '../internal/newObjectWithoutPrototypeProperties';
 
 const slice = [].slice;
 
@@ -58,7 +58,7 @@ Compiler.prototype = {
     // These changes will propagate to the other compiler components
     let knownHelpers = options.knownHelpers;
 
-    options.knownHelpers = newMapObject({
+    options.knownHelpers = newObjectWithoutPrototypeProperties({
       'helperMissing': true,
       'blockHelperMissing': true,
       'each': true,

lib/handlebars/compiler/javascript-compiler.js

@@ -2,7 +2,7 @@ import { COMPILER_REVISION, REVISION_CHANGES } from '../base';
 import Exception from '../exception';
 import {isArray} from '../utils';
 import CodeGen from './code-gen';
-import {newMapObject} from '../internal/utils';
+import {propertyMustBeEnumerable} from '../internal/propertyMustBeEnumerable';
 
 function Literal(value) {
   this.value = value;
@@ -16,7 +16,7 @@ JavaScriptCompiler.prototype = {
   nameLookup: function(parent, name/* , type*/) {
     const isEnumerable = [ this.aliasable('container.propertyIsEnumerable'), '.call(', parent, ',', JSON.stringify(name), ')'];
 
-    if (this.propertyMustBeEnumerable[name]) {
+    if (propertyMustBeEnumerable(name)) {
       return ['(', isEnumerable, '?', _actualLookup(), ' : undefined)'];
     }
     return _actualLookup();
@@ -96,14 +96,6 @@ JavaScriptCompiler.prototype = {
     this.useBlockParams = this.useBlockParams || environment.useBlockParams;
     this.allowNonHelperFunctionCall = this.options.allowNonHelperFunctionCall !== false; // default if true for 4.x
 
-    // Workaround: We cannot use {...}-notation to create this object, because that way, we cannot overwrite __proto__
-    let propertyMustBeEnumerableDefaultValues = Object.create(null);
-    ['__defineGetter__', '__defineSetter__', '__proto__', 'constructor'].forEach((forbiddenProperty) => {
-      propertyMustBeEnumerableDefaultValues[forbiddenProperty] = true;
-    });
-
-    this.propertyMustBeEnumerable = newMapObject(propertyMustBeEnumerableDefaultValues, this.options.propertyMustBeEnumerable);
-
     let opcodes = environment.opcodes,
         opcode,
         firstLoc,
@@ -1027,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,7 +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 {newMapObject} from './internal/utils';
+import {newObjectWithoutPrototypeProperties} from './internal/newObjectWithoutPrototypeProperties';
 
 export function checkRevision(compilerInfo) {
   const compilerRevision = compilerInfo && compilerInfo[0] || 1,
@@ -159,13 +159,13 @@ export function template(templateSpec, env) {
 
   ret._setup = function(options) {
     if (!options.partial) {
-      container.helpers = newMapObject(env.helpers, options.helpers);
+      container.helpers = newObjectWithoutPrototypeProperties(env.helpers, options.helpers);
 
       if (templateSpec.usePartial) {
-        container.partials = newMapObject(env.partials, options.partials);
+        container.partials = newObjectWithoutPrototypeProperties(env.partials, options.partials);
       }
       if (templateSpec.usePartial || templateSpec.useDecorators) {
-        container.decorators = newMapObject(env.decorators, options.decorators);
+        container.decorators = newObjectWithoutPrototypeProperties(env.decorators, options.decorators);
       }
 
       container.hooks = {};

types/index.d.ts

@@ -215,11 +215,6 @@ interface CompileOptions {
   ignoreStandalone?: boolean;
   explicitPartialContext?: boolean;
   allowNonHelperFunctionCall?: boolean;
-  propertyMustBeEnumerable?: PropertyMustBeEnumerable;
-}
-
-type PropertyMustBeEnumerable = {
-  [name: string]: boolean;
 }
 
 type KnownHelpers = {