Skip to content

Commit

Permalink
Disallow calling "helperMissing" and "blockHelperMissing" directly
Browse files Browse the repository at this point in the history
closes #1558
  • Loading branch information
nknapp committed Sep 24, 2019
1 parent fff3e40 commit 2078c72
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 25 deletions.
32 changes: 23 additions & 9 deletions lib/handlebars/compiler/javascript-compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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..., ...
Expand Down Expand Up @@ -673,7 +687,7 @@ JavaScriptCompiler.prototype = {
lookup[0] = '(helper = ';
lookup.push(
' != null ? helper : ',
this.aliasable('helpers.helperMissing')
this.aliasable('container.hooks.helperMissing')
);
}

Expand Down
9 changes: 9 additions & 0 deletions lib/handlebars/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
}
}
35 changes: 19 additions & 16 deletions lib/handlebars/runtime.js
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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');
Expand All @@ -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) {
Expand Down Expand Up @@ -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({}),

Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions lib/handlebars/utils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

const escape = {
'&': '&amp;',
'<': '&lt;',
Expand Down
56 changes: 56 additions & 0 deletions spec/security.js
Original file line number Diff line number Diff line change
Expand Up @@ -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});
});
});
});
});
1 change: 1 addition & 0 deletions types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ declare namespace Handlebars {
decorators?: { [name: string]: Function };
data?: any;
blockParams?: any[];
allowCallsToHelperMissing: boolean;
}

export interface HelperOptions {
Expand Down

0 comments on commit 2078c72

Please sign in to comment.