From 2078c727c627f25d4a149962f05c1e069beb18bc Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Mon, 16 Sep 2019 23:42:09 +0200 Subject: [PATCH] Disallow calling "helperMissing" and "blockHelperMissing" directly closes #1558 --- .../compiler/javascript-compiler.js | 32 ++++++++--- lib/handlebars/helpers.js | 9 +++ lib/handlebars/runtime.js | 35 ++++++------ lib/handlebars/utils.js | 1 + spec/security.js | 56 +++++++++++++++++++ types/index.d.ts | 1 + 6 files changed, 109 insertions(+), 25 deletions(-) diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index ff98ad9e4..1b9b2318d 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -311,7 +311,7 @@ JavaScriptCompiler.prototype = { // replace it on the stack with the result of properly // invoking blockHelperMissing. blockValue: function(name) { - let blockHelperMissing = this.aliasable('helpers.blockHelperMissing'), + let blockHelperMissing = this.aliasable('container.hooks.blockHelperMissing'), params = [this.contextName(0)]; this.setupHelperArgs(name, 0, params); @@ -329,7 +329,7 @@ JavaScriptCompiler.prototype = { // On stack, after, if lastHelper: value ambiguousBlockValue: function() { // We're being a bit cheeky and reusing the options value from the prior exec - let blockHelperMissing = this.aliasable('helpers.blockHelperMissing'), + let blockHelperMissing = this.aliasable('container.hooks.blockHelperMissing'), params = [this.contextName(0)]; this.setupHelperArgs('', 0, params, true); @@ -622,18 +622,32 @@ JavaScriptCompiler.prototype = { // If the helper is not found, `helperMissing` is called. invokeHelper: function(paramSize, name, isSimple) { let nonHelper = this.popStack(), - helper = this.setupHelper(paramSize, name), - simple = isSimple ? [helper.name, ' || '] : ''; + helper = this.setupHelper(paramSize, name); - let lookup = ['('].concat(simple, nonHelper); + let possibleFunctionCalls = []; + + if (isSimple) { // direct call to helper + possibleFunctionCalls.push(helper.name); + } + // call a function from the input object + possibleFunctionCalls.push(nonHelper); if (!this.options.strict) { - lookup.push(' || ', this.aliasable('helpers.helperMissing')); + possibleFunctionCalls.push(this.aliasable('container.hooks.helperMissing')); } - lookup.push(')'); - this.push(this.source.functionCall(lookup, 'call', helper.callParams)); + let functionLookupCode = ['(', this.itemsSeparatedBy(possibleFunctionCalls, '||'), ')']; + let functionCall = this.source.functionCall(functionLookupCode, 'call', helper.callParams); + this.push(functionCall); }, + itemsSeparatedBy: function(items, separator) { + let result = []; + result.push(items[0]); + for (let i = 1; i < items.length; i++) { + result.push(separator, items[i]); + } + return result; + }, // [invokeKnownHelper] // // On stack, before: hash, inverse, program, params..., ... @@ -673,7 +687,7 @@ JavaScriptCompiler.prototype = { lookup[0] = '(helper = '; lookup.push( ' != null ? helper : ', - this.aliasable('helpers.helperMissing') + this.aliasable('container.hooks.helperMissing') ); } diff --git a/lib/handlebars/helpers.js b/lib/handlebars/helpers.js index 7a4365aea..804796820 100644 --- a/lib/handlebars/helpers.js +++ b/lib/handlebars/helpers.js @@ -15,3 +15,12 @@ export function registerDefaultHelpers(instance) { registerLookup(instance); registerWith(instance); } + +export function moveHelperToHooks(instance, helperName, keepHelper) { + if (instance.helpers[helperName]) { + instance.hooks[helperName] = instance.helpers[helperName]; + if (!keepHelper) { + delete instance.helpers[helperName]; + } + } +} diff --git a/lib/handlebars/runtime.js b/lib/handlebars/runtime.js index a00f71e06..ce4630463 100644 --- a/lib/handlebars/runtime.js +++ b/lib/handlebars/runtime.js @@ -1,6 +1,7 @@ import * as Utils from './utils'; import Exception from './exception'; -import { COMPILER_REVISION, REVISION_CHANGES, createFrame } from './base'; +import {COMPILER_REVISION, createFrame, REVISION_CHANGES} from './base'; +import {moveHelperToHooks} from './helpers'; export function checkRevision(compilerInfo) { const compilerRevision = compilerInfo && compilerInfo[0] || 1, @@ -21,6 +22,7 @@ export function checkRevision(compilerInfo) { } export function template(templateSpec, env) { + /* istanbul ignore next */ if (!env) { throw new Exception('No environment passed to template'); @@ -42,13 +44,15 @@ export function template(templateSpec, env) { options.ids[0] = true; } } - partial = env.VM.resolvePartial.call(this, partial, context, options); - let result = env.VM.invokePartial.call(this, partial, context, options); + + let optionsWithHooks = Utils.extend({}, options, {hooks: this.hooks}); + + let result = env.VM.invokePartial.call(this, partial, context, optionsWithHooks); if (result == null && env.compile) { options.partials[options.name] = env.compile(partial, templateSpec.compilerOptions, env); - result = options.partials[options.name](context, options); + result = options.partials[options.name](context, optionsWithHooks); } if (result != null) { if (options.indent) { @@ -115,15 +119,6 @@ export function template(templateSpec, env) { } return value; }, - merge: function(param, common) { - let obj = param || common; - - if (param && common && (param !== common)) { - obj = Utils.extend({}, common, param); - } - - return obj; - }, // An empty object to use as replacement for null-contexts nullContext: Object.seal({}), @@ -158,19 +153,27 @@ export function template(templateSpec, env) { ret._setup = function(options) { if (!options.partial) { - container.helpers = container.merge(options.helpers, env.helpers); + container.helpers = Utils.extend({}, env.helpers, options.helpers); if (templateSpec.usePartial) { - container.partials = container.merge(options.partials, env.partials); + container.partials = Utils.extend({}, env.partials, options.partials); } if (templateSpec.usePartial || templateSpec.useDecorators) { - container.decorators = container.merge(options.decorators, env.decorators); + container.decorators = Utils.extend({}, env.decorators, options.decorators); } + + container.hooks = {}; + let keepHelper = options.allowCallsToHelperMissing; + moveHelperToHooks(container, 'helperMissing', keepHelper); + moveHelperToHooks(container, 'blockHelperMissing', keepHelper); + } else { container.helpers = options.helpers; container.partials = options.partials; container.decorators = options.decorators; + container.hooks = options.hooks; } + }; ret._child = function(i, data, blockParams, depths) { diff --git a/lib/handlebars/utils.js b/lib/handlebars/utils.js index 2584601ef..8e1a4014f 100644 --- a/lib/handlebars/utils.js +++ b/lib/handlebars/utils.js @@ -1,3 +1,4 @@ + const escape = { '&': '&', '<': '<', diff --git a/spec/security.js b/spec/security.js index 4a7d83773..4c092b1c0 100644 --- a/spec/security.js +++ b/spec/security.js @@ -32,4 +32,60 @@ describe('security issues', function() { }); }); + + describe('GH-xxxx: Prevent explicit call of helperMissing-helpers', function() { + if (!Handlebars.compile) { + return; + } + + describe('without the option "allowExplicitCallOfHelperMissing"', function() { + it('should throw an exception when calling "{{helperMissing}}" ', function() { + shouldThrow(function() { + var template = Handlebars.compile('{{helperMissing}}'); + template({}); + }, Error); + }); + it('should throw an exception when calling "{{#helperMissing}}{{/helperMissing}}" ', function() { + shouldThrow(function() { + var template = Handlebars.compile('{{#helperMissing}}{{/helperMissing}}'); + template({}); + }, Error); + }); + it('should throw an exception when calling "{{blockHelperMissing "abc" .}}" ', function() { + var functionCalls = []; + shouldThrow(function() { + var template = Handlebars.compile('{{blockHelperMissing "abc" .}}'); + template({ fn: function() { functionCalls.push('called'); }}); + }, Error); + equals(functionCalls.length, 0); + }); + it('should throw an exception when calling "{{#blockHelperMissing .}}{{/blockHelperMissing}}"', function() { + shouldThrow(function() { + var template = Handlebars.compile('{{#blockHelperMissing .}}{{/blockHelperMissing}}'); + template({ fn: function() { return 'functionInData';}}); + }, Error); + }); + }); + + describe('with the option "allowCallsToHelperMissing" set to true', function() { + it('should not throw an exception when calling "{{helperMissing}}" ', function() { + var template = Handlebars.compile('{{helperMissing}}'); + template({}, {allowCallsToHelperMissing: true}); + }); + it('should not throw an exception when calling "{{#helperMissing}}{{/helperMissing}}" ', function() { + var template = Handlebars.compile('{{#helperMissing}}{{/helperMissing}}'); + template({}, {allowCallsToHelperMissing: true}); + }); + it('should not throw an exception when calling "{{blockHelperMissing "abc" .}}" ', function() { + var functionCalls = []; + var template = Handlebars.compile('{{blockHelperMissing "abc" .}}'); + template({ fn: function() { functionCalls.push('called'); }}, {allowCallsToHelperMissing: true}); + equals(functionCalls.length, 1); + }); + it('should not throw an exception when calling "{{#blockHelperMissing .}}{{/blockHelperMissing}}"', function() { + var template = Handlebars.compile('{{#blockHelperMissing true}}sdads{{/blockHelperMissing}}'); + template({}, {allowCallsToHelperMissing: true}); + }); + }); + }); }); diff --git a/types/index.d.ts b/types/index.d.ts index a142119a4..bf8d17564 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -29,6 +29,7 @@ declare namespace Handlebars { decorators?: { [name: string]: Function }; data?: any; blockParams?: any[]; + allowCallsToHelperMissing: boolean; } export interface HelperOptions {