diff --git a/Gruntfile.js b/Gruntfile.js index 2a0fea65..8b953832 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -215,7 +215,7 @@ module.exports = function(grunt) { grunt.registerTask('bench', ['metrics']); - if (process.env.SAUCE_USERNAME) { + if (process.env.SAUCE_ACCESS_KEY) { grunt.registerTask('sauce', ['concat:tests', 'connect', 'saucelabs-mocha']); } else { grunt.registerTask('sauce', []); diff --git a/lib/handlebars/base.js b/lib/handlebars/base.js index 88ba0ddc..40f2fc56 100644 --- a/lib/handlebars/base.js +++ b/lib/handlebars/base.js @@ -3,8 +3,9 @@ import Exception from './exception'; import { registerDefaultHelpers } from './helpers'; import { registerDefaultDecorators } from './decorators'; import logger from './logger'; +import { resetLoggedProperties } from './internal/proto-access'; -export const VERSION = '4.6.0'; +export const VERSION = '4.7.2'; export const COMPILER_REVISION = 8; export const LAST_COMPATIBLE_COMPILER_REVISION = 7; @@ -78,6 +79,13 @@ HandlebarsEnvironment.prototype = { }, unregisterDecorator: function(name) { delete this.decorators[name]; + }, + /** + * Reset the memory of illegal property accesses that have already been logged. + * @deprecated should only be used in handlebars test-cases + */ + resetLoggedPropertyAccesses() { + resetLoggedProperties(); } }; diff --git a/lib/handlebars/internal/createNewLookupObject.js b/lib/handlebars/internal/create-new-lookup-object.js similarity index 100% rename from lib/handlebars/internal/createNewLookupObject.js rename to lib/handlebars/internal/create-new-lookup-object.js diff --git a/lib/handlebars/internal/proto-access.js b/lib/handlebars/internal/proto-access.js new file mode 100644 index 00000000..a8c5394d --- /dev/null +++ b/lib/handlebars/internal/proto-access.js @@ -0,0 +1,70 @@ +import { createNewLookupObject } from './create-new-lookup-object'; +import * as logger from '../logger'; + +const loggedProperties = Object.create(null); + +export function createProtoAccessControl(runtimeOptions) { + let defaultMethodWhiteList = Object.create(null); + defaultMethodWhiteList['constructor'] = false; + defaultMethodWhiteList['__defineGetter__'] = false; + defaultMethodWhiteList['__defineSetter__'] = false; + defaultMethodWhiteList['__lookupGetter__'] = false; + + let defaultPropertyWhiteList = Object.create(null); + // eslint-disable-next-line no-proto + defaultPropertyWhiteList['__proto__'] = false; + + return { + properties: { + whitelist: createNewLookupObject( + defaultPropertyWhiteList, + runtimeOptions.allowedProtoProperties + ), + defaultValue: runtimeOptions.allowProtoPropertiesByDefault + }, + methods: { + whitelist: createNewLookupObject( + defaultMethodWhiteList, + runtimeOptions.allowedProtoMethods + ), + defaultValue: runtimeOptions.allowProtoMethodsByDefault + } + }; +} + +export function resultIsAllowed(result, protoAccessControl, propertyName) { + if (typeof result === 'function') { + return checkWhiteList(protoAccessControl.methods, propertyName); + } else { + return checkWhiteList(protoAccessControl.properties, propertyName); + } +} + +function checkWhiteList(protoAccessControlForType, propertyName) { + if (protoAccessControlForType.whitelist[propertyName] !== undefined) { + return protoAccessControlForType.whitelist[propertyName] === true; + } + if (protoAccessControlForType.defaultValue !== undefined) { + return protoAccessControlForType.defaultValue; + } + logUnexpecedPropertyAccessOnce(propertyName); + return false; +} + +function logUnexpecedPropertyAccessOnce(propertyName) { + if (loggedProperties[propertyName] !== true) { + loggedProperties[propertyName] = true; + logger.log( + 'error', + `Handlebars: Access has been denied to resolve the property "${propertyName}" because it is not an "own property" of its parent.\n` + + `You can add a runtime option to disable the check or this warning:\n` + + `See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details` + ); + } +} + +export function resetLoggedProperties() { + Object.keys(loggedProperties).forEach(propertyName => { + delete loggedProperties[propertyName]; + }); +} diff --git a/lib/handlebars/internal/wrapHelper.js b/lib/handlebars/internal/wrapHelper.js index 0010eb8c..29d65b03 100644 --- a/lib/handlebars/internal/wrapHelper.js +++ b/lib/handlebars/internal/wrapHelper.js @@ -1,4 +1,9 @@ export function wrapHelper(helper, transformOptionsFn) { + if (typeof helper !== 'function') { + // This should not happen, but apparently it does in https://github.com/wycats/handlebars.js/issues/1639 + // We try to make the wrapper least-invasive by not wrapping it, if the helper is not a function. + return helper; + } let wrapper = function(/* dynamic arguments */) { const options = arguments[arguments.length - 1]; arguments[arguments.length - 1] = transformOptionsFn(options); diff --git a/lib/handlebars/runtime.js b/lib/handlebars/runtime.js index c31249cb..cb3ff3d0 100644 --- a/lib/handlebars/runtime.js +++ b/lib/handlebars/runtime.js @@ -8,7 +8,10 @@ import { } from './base'; import { moveHelperToHooks } from './helpers'; import { wrapHelper } from './internal/wrapHelper'; -import { createNewLookupObject } from './internal/createNewLookupObject'; +import { + createProtoAccessControl, + resultIsAllowed +} from './internal/proto-access'; export function checkRevision(compilerInfo) { const compilerRevision = (compilerInfo && compilerInfo[0]) || 1, @@ -70,8 +73,7 @@ export function template(templateSpec, env) { let extendedOptions = Utils.extend({}, options, { hooks: this.hooks, - allowedProtoMethods: this.allowedProtoMethods, - allowedProtoProperties: this.allowedProtoProperties + protoAccessControl: this.protoAccessControl }); let result = env.VM.invokePartial.call( @@ -123,15 +125,14 @@ export function template(templateSpec, env) { }, lookupProperty: function(parent, propertyName) { let result = parent[propertyName]; + if (result == null) { + return result; + } if (Object.prototype.hasOwnProperty.call(parent, propertyName)) { return result; } - const whitelist = - typeof result === 'function' - ? container.allowedProtoMethods - : container.allowedProtoProperties; - if (whitelist[propertyName] === true) { + if (resultIsAllowed(result, container.protoAccessControl, propertyName)) { return result; } return undefined; @@ -234,6 +235,7 @@ export function template(templateSpec, env) { ) ); } + main = executeDecorators( templateSpec.main, main, @@ -244,6 +246,7 @@ export function template(templateSpec, env) { ); return main(context, options); } + ret.isTop = true; function _setup(options) { @@ -268,12 +271,7 @@ export function template(templateSpec, env) { } container.hooks = {}; - container.allowedProtoProperties = createNewLookupObject( - options.allowedProtoProperties - ); - container.allowedProtoMethods = createNewLookupObject( - options.allowedProtoMethods - ); + container.protoAccessControl = createProtoAccessControl(options); let keepHelperInHelpers = options.allowCallsToHelperMissing || @@ -281,8 +279,7 @@ export function template(templateSpec, env) { moveHelperToHooks(container, 'helperMissing', keepHelperInHelpers); moveHelperToHooks(container, 'blockHelperMissing', keepHelperInHelpers); } else { - container.allowedProtoProperties = options.allowedProtoProperties; - container.allowedProtoMethods = options.allowedProtoMethods; + container.protoAccessControl = options.protoAccessControl; // internal option container.helpers = options.helpers; container.partials = options.partials; container.decorators = options.decorators; diff --git a/release-notes.md b/release-notes.md index d75b3202..09ecc2a0 100644 --- a/release-notes.md +++ b/release-notes.md @@ -2,7 +2,51 @@ ## Development -[Commits](https://github.com/wycats/handlebars.js/compare/v4.6.0...master) +[Commits](https://github.com/wycats/handlebars.js/compare/v4.7.2...master) + +## v4.7.2 - January 13th, 2020 + +Bugfixes: + +- fix: don't wrap helpers that are not functions - 9d5aa36, #1639 + +Chore/Build: + +- chore: execute saucelabs-task only if access-key exists - a4fd391 + +Compatibility notes: + +- No breaking changes are to be expected + +[Commits](https://github.com/wycats/handlebars.js/compare/v4.7.1...v4.7.2) + +## v4.7.1 - January 12th, 2020 + +Bugfixes: + +- fix: fix log output in case of illegal property access - f152dfc +- fix: log error for illegal property access only once per property - 3c1e252 + +Compatibility notes: + +- no incompatibilities are to be expected. + +[Commits](https://github.com/wycats/handlebars.js/compare/v4.7.0...v4.7.1) + +## v4.7.0 - January 10th, 2020 + +Features: + +- feat: default options for controlling proto access - 7af1c12, #1635 + - This makes it possible to disable the prototype access restrictions added in 4.6.0 + - an error is logged in the console, if access to prototype properties is attempted and denied + and no explicit configuration has taken place. + +Compatibility notes: + +- no compatibilities are expected + +[Commits](https://github.com/wycats/handlebars.js/compare/v4.6.0...v4.7.0) ## v4.6.0 - January 8th, 2020 diff --git a/spec/regressions.js b/spec/regressions.js index f6dc3512..12e6f5e2 100644 --- a/spec/regressions.js +++ b/spec/regressions.js @@ -529,4 +529,13 @@ describe('Regressions', function() { sinon.restore(); }); }); + + describe("GH-1639: TypeError: Cannot read property 'apply' of undefined\" when handlebars version > 4.6.0 (undocumented, deprecated usage)", function() { + it('should treat undefined helpers like non-existing helpers', function() { + expectTemplate('{{foo}}') + .withHelper('foo', undefined) + .withInput({ foo: 'bar' }) + .toCompileTo('bar'); + }); + }); }); diff --git a/spec/security.js b/spec/security.js index 7bf9d89a..1b345f0c 100644 --- a/spec/security.js +++ b/spec/security.js @@ -149,176 +149,272 @@ describe('security issues', function() { }); }); - describe('GH-1595', function() { - it('properties, that are required to be own properties', function() { - expectTemplate('{{constructor}}') - .withInput({}) - .toCompileTo(''); - - expectTemplate('{{__defineGetter__}}') - .withInput({}) - .toCompileTo(''); + describe('GH-1595: dangerous properties', function() { + var templates = [ + '{{constructor}}', + '{{__defineGetter__}}', + '{{__defineSetter__}}', + '{{__lookupGetter__}}', + '{{__proto__}}', + '{{lookup this "constructor"}}', + '{{lookup this "__defineGetter__"}}', + '{{lookup this "__defineSetter__"}}', + '{{lookup this "__lookupGetter__"}}', + '{{lookup this "__proto__"}}' + ]; + + templates.forEach(function(template) { + describe('access should be denied to ' + template, function() { + it('by default', function() { + expectTemplate(template) + .withInput({}) + .toCompileTo(''); + }); + it(' with proto-access enabled', function() { + expectTemplate(template) + .withInput({}) + .withRuntimeOptions({ + allowProtoPropertiesByDefault: true, + allowProtoMethodsByDefault: true + }) + .toCompileTo(''); + }); + }); + }); + }); + describe('GH-1631: disallow access to prototype functions', function() { + function TestClass() {} - expectTemplate('{{__defineSetter__}}') - .withInput({}) - .toCompileTo(''); + TestClass.prototype.aProperty = 'propertyValue'; + TestClass.prototype.aMethod = function() { + return 'returnValue'; + }; - expectTemplate('{{__lookupGetter__}}') - .withInput({}) - .toCompileTo(''); + beforeEach(function() { + handlebarsEnv.resetLoggedPropertyAccesses(); + }); - expectTemplate('{{__proto__}}') - .withInput({}) - .toCompileTo(''); + afterEach(function() { + sinon.restore(); + }); - expectTemplate('{{lookup this "constructor"}}') - .withInput({}) - .toCompileTo(''); + describe('control access to prototype methods via "allowedProtoMethods"', function() { + checkProtoMethodAccess({}); - expectTemplate('{{lookup this "__defineGetter__"}}') - .withInput({}) - .toCompileTo(''); + describe('in compat mode', function() { + checkProtoMethodAccess({ compat: true }); + }); - expectTemplate('{{lookup this "__defineSetter__"}}') - .withInput({}) - .toCompileTo(''); + function checkProtoMethodAccess(compileOptions) { + it('should be prohibited by default and log a warning', function() { + var spy = sinon.spy(console, 'error'); - expectTemplate('{{lookup this "__lookupGetter__"}}') - .withInput({}) - .toCompileTo(''); + expectTemplate('{{aMethod}}') + .withInput(new TestClass()) + .withCompileOptions(compileOptions) + .toCompileTo(''); - expectTemplate('{{lookup this "__proto__"}}') - .withInput({}) - .toCompileTo(''); - }); + expect(spy.calledOnce).to.be.true(); + expect(spy.args[0][0]).to.match(/Handlebars: Access has been denied/); + }); - describe('GH-1631: disallow access to prototype functions', function() { - function TestClass() {} + it('should only log the warning once', function() { + var spy = sinon.spy(console, 'error'); - TestClass.prototype.aProperty = 'propertyValue'; - TestClass.prototype.aMethod = function() { - return 'returnValue'; - }; + expectTemplate('{{aMethod}}') + .withInput(new TestClass()) + .withCompileOptions(compileOptions) + .toCompileTo(''); - describe('control access to prototype methods via "allowedProtoMethods"', function() { - it('should be prohibited by default', function() { expectTemplate('{{aMethod}}') .withInput(new TestClass()) + .withCompileOptions(compileOptions) .toCompileTo(''); + + expect(spy.calledOnce).to.be.true(); + expect(spy.args[0][0]).to.match(/Handlebars: Access has been denied/); }); - it('can be allowed', function() { + it('can be allowed, which disables the warning', function() { + var spy = sinon.spy(console, 'error'); + expectTemplate('{{aMethod}}') .withInput(new TestClass()) + .withCompileOptions(compileOptions) .withRuntimeOptions({ allowedProtoMethods: { aMethod: true } }) .toCompileTo('returnValue'); + + expect(spy.callCount).to.equal(0); }); - it('should be prohibited by default (in "compat" mode)', function() { + it('can be turned on by default, which disables the warning', function() { + var spy = sinon.spy(console, 'error'); + expectTemplate('{{aMethod}}') .withInput(new TestClass()) - .withCompileOptions({ compat: true }) - .toCompileTo(''); + .withCompileOptions(compileOptions) + .withRuntimeOptions({ + allowProtoMethodsByDefault: true + }) + .toCompileTo('returnValue'); + + expect(spy.callCount).to.equal(0); }); - it('can be allowed (in "compat" mode)', function() { + it('can be turned off by default, which disables the warning', function() { + var spy = sinon.spy(console, 'error'); + expectTemplate('{{aMethod}}') .withInput(new TestClass()) - .withCompileOptions({ compat: true }) + .withCompileOptions(compileOptions) .withRuntimeOptions({ - allowedProtoMethods: { - aMethod: true - } + allowProtoMethodsByDefault: false }) - .toCompileTo('returnValue'); - }); + .toCompileTo(''); - it('should cause the recursive lookup by default (in "compat" mode)', function() { - expectTemplate('{{#aString}}{{trim}}{{/aString}}') - .withInput({ aString: ' abc ', trim: 'trim' }) - .withCompileOptions({ compat: true }) - .toCompileTo('trim'); + expect(spy.callCount).to.equal(0); }); - it('should not cause the recursive lookup if allowed through options(in "compat" mode)', function() { - expectTemplate('{{#aString}}{{trim}}{{/aString}}') - .withInput({ aString: ' abc ', trim: 'trim' }) - .withCompileOptions({ compat: true }) + it('can be turned off, if turned on by default', function() { + expectTemplate('{{aMethod}}') + .withInput(new TestClass()) + .withCompileOptions(compileOptions) .withRuntimeOptions({ + allowProtoMethodsByDefault: true, allowedProtoMethods: { - trim: true + aMethod: false } }) - .toCompileTo('abc'); + .toCompileTo(''); }); + } + + it('should cause the recursive lookup by default (in "compat" mode)', function() { + expectTemplate('{{#aString}}{{trim}}{{/aString}}') + .withInput({ aString: ' abc ', trim: 'trim' }) + .withCompileOptions({ compat: true }) + .toCompileTo('trim'); + }); + + it('should not cause the recursive lookup if allowed through options(in "compat" mode)', function() { + expectTemplate('{{#aString}}{{trim}}{{/aString}}') + .withInput({ aString: ' abc ', trim: 'trim' }) + .withCompileOptions({ compat: true }) + .withRuntimeOptions({ + allowedProtoMethods: { + trim: true + } + }) + .toCompileTo('abc'); + }); + }); + + describe('control access to prototype non-methods via "allowedProtoProperties" and "allowProtoPropertiesByDefault', function() { + checkProtoPropertyAccess({}); + + describe('in compat-mode', function() { + checkProtoPropertyAccess({ compat: true }); }); - describe('control access to prototype non-methods via "allowedProtoProperties"', function() { - it('should be prohibited by default', function() { + function checkProtoPropertyAccess(compileOptions) { + it('should be prohibited by default and log a warning', function() { + var spy = sinon.spy(console, 'error'); + expectTemplate('{{aProperty}}') .withInput(new TestClass()) + .withCompileOptions(compileOptions) .toCompileTo(''); + + expect(spy.calledOnce).to.be.true(); + expect(spy.args[0][0]).to.match(/Handlebars: Access has been denied/); }); - it('can be turned on', function() { + it('can be explicitly prohibited by default, which disables the warning', function() { + var spy = sinon.spy(console, 'error'); + expectTemplate('{{aProperty}}') .withInput(new TestClass()) + .withCompileOptions(compileOptions) + .withRuntimeOptions({ + allowProtoPropertiesByDefault: false + }) + .toCompileTo(''); + + expect(spy.callCount).to.equal(0); + }); + + it('can be turned on, which disables the warning', function() { + var spy = sinon.spy(console, 'error'); + + expectTemplate('{{aProperty}}') + .withInput(new TestClass()) + .withCompileOptions(compileOptions) .withRuntimeOptions({ allowedProtoProperties: { aProperty: true } }) .toCompileTo('propertyValue'); + + expect(spy.callCount).to.equal(0); }); - it('should be prohibited by default (in "compat" mode)', function() { + it('can be turned on by default, which disables the warning', function() { + var spy = sinon.spy(console, 'error'); + expectTemplate('{{aProperty}}') .withInput(new TestClass()) - .withCompileOptions({ compat: true }) - .toCompileTo(''); + .withCompileOptions(compileOptions) + .withRuntimeOptions({ + allowProtoPropertiesByDefault: true + }) + .toCompileTo('propertyValue'); + + expect(spy.callCount).to.equal(0); }); - it('can be turned on (in "compat" mode)', function() { + it('can be turned off, if turned on by default', function() { expectTemplate('{{aProperty}}') .withInput(new TestClass()) - .withCompileOptions({ compat: true }) + .withCompileOptions(compileOptions) .withRuntimeOptions({ + allowProtoPropertiesByDefault: true, allowedProtoProperties: { - aProperty: true + aProperty: false } }) - .toCompileTo('propertyValue'); + .toCompileTo(''); }); - }); + } + }); - describe('compatibility with old runtimes, that do not provide the function "container.lookupProperty"', function() { - beforeEach(function simulateRuntimeWithoutLookupProperty() { - var oldTemplateMethod = handlebarsEnv.template; - sinon.replace(handlebarsEnv, 'template', function(templateSpec) { - templateSpec.main = wrapToAdjustContainer(templateSpec.main); - return oldTemplateMethod.call(this, templateSpec); - }); + describe('compatibility with old runtimes, that do not provide the function "container.lookupProperty"', function() { + beforeEach(function simulateRuntimeWithoutLookupProperty() { + var oldTemplateMethod = handlebarsEnv.template; + sinon.replace(handlebarsEnv, 'template', function(templateSpec) { + templateSpec.main = wrapToAdjustContainer(templateSpec.main); + return oldTemplateMethod.call(this, templateSpec); }); + }); - afterEach(function() { - sinon.restore(); - }); + afterEach(function() { + sinon.restore(); + }); - it('should work with simple properties', function() { - expectTemplate('{{aProperty}}') - .withInput({ aProperty: 'propertyValue' }) - .toCompileTo('propertyValue'); - }); + it('should work with simple properties', function() { + expectTemplate('{{aProperty}}') + .withInput({ aProperty: 'propertyValue' }) + .toCompileTo('propertyValue'); + }); - it('should work with Array.prototype.length', function() { - expectTemplate('{{anArray.length}}') - .withInput({ anArray: ['a', 'b', 'c'] }) - .toCompileTo('3'); - }); + it('should work with Array.prototype.length', function() { + expectTemplate('{{anArray.length}}') + .withInput({ anArray: ['a', 'b', 'c'] }) + .toCompileTo('3'); }); }); }); diff --git a/types/index.d.ts b/types/index.d.ts index 606741f5..1fa83680 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -30,8 +30,10 @@ declare namespace Handlebars { data?: any; blockParams?: any[]; allowCallsToHelperMissing?: boolean; - allowedProtoProperties?: { [name: string]: boolean } - allowedProtoMethods?: { [name: string]: boolean } + allowedProtoProperties?: { [name: string]: boolean }; + allowedProtoMethods?: { [name: string]: boolean }; + allowProtoPropertiesByDefault?: boolean; + allowProtoMethodsByDefault?: boolean; } export interface HelperOptions { diff --git a/types/test.ts b/types/test.ts index b081ae4f..7a34b7eb 100644 --- a/types/test.ts +++ b/types/test.ts @@ -241,12 +241,14 @@ function testExceptionWithNodeTypings() { let stack: string | undefined = exception.stack; } -function testProtoPropertyControlOptions() { +function testProtoAccessControlControlOptions() { Handlebars.compile('test')( {}, { allowedProtoMethods: { allowedMethod: true, forbiddenMethod: false }, - allowedProtoProperties: { allowedProperty: true, forbiddenProperty: false } + allowedProtoProperties: { allowedProperty: true, forbiddenProperty: false }, + allowProtoMethodsByDefault: true, + allowProtoPropertiesByDefault: false, } ); }