From 7af1c12db6222ab4b689bb60820628209d295049 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Thu, 9 Jan 2020 23:00:32 +0100 Subject: [PATCH 01/12] feat: default options for controlling proto access This commmit adds the runtime options - `allowProtoPropertiesByDefault` (boolean, default: false) and - `allowProtoMethodsByDefault` (boolean, default: false)` which can be used to allow access to prototype properties and functions in general. Specific properties and methods can still be disabled from access via `allowedProtoProperties` and `allowedProtoMethods` by setting the corresponding values to false. The methods `constructor`, `__defineGetter__`, `__defineSetter__`, `__lookupGetter__` and the property `__proto__` will be disabled, even if the allow...ByDefault-options are set to true. In order to allow access to those properties and methods, they have to be explicitly set to true in the 'allowedProto...'-options. A warning is logged when the a proto-access it attempted and denied by default (i.e. if no option is set by the user to make the access decision explicit) --- ...pObject.js => create-new-lookup-object.js} | 0 lib/handlebars/internal/proto-access.js | 55 ++++ lib/handlebars/runtime.js | 29 +- spec/security.js | 275 +++++++++++------- types/index.d.ts | 6 +- types/test.ts | 6 +- 6 files changed, 251 insertions(+), 120 deletions(-) rename lib/handlebars/internal/{createNewLookupObject.js => create-new-lookup-object.js} (100%) create mode 100644 lib/handlebars/internal/proto-access.js 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..59bf1444 --- /dev/null +++ b/lib/handlebars/internal/proto-access.js @@ -0,0 +1,55 @@ +import { createNewLookupObject } from './create-new-lookup-object'; + +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; + } + + // eslint-disable-next-line no-console + console.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 http://localhost:8080/api-reference/runtime-options.html#options-to-control-prototype-access for details` + ); + return false; +} diff --git a/lib/handlebars/runtime.js b/lib/handlebars/runtime.js index 7d63e87b..67a500d7 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, @@ -73,8 +76,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( @@ -126,15 +128,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; @@ -237,6 +238,7 @@ export function template(templateSpec, env) { ) ); } + main = executeDecorators( templateSpec.main, main, @@ -247,6 +249,7 @@ export function template(templateSpec, env) { ); return main(context, options); } + ret.isTop = true; ret._setup = function(options) { @@ -271,12 +274,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 || @@ -284,8 +282,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/spec/security.js b/spec/security.js index 7bf9d89a..bf0be148 100644 --- a/spec/security.js +++ b/spec/security.js @@ -149,176 +149,251 @@ 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(''); - - expectTemplate('{{__defineSetter__}}') - .withInput({}) - .toCompileTo(''); - - expectTemplate('{{__lookupGetter__}}') - .withInput({}) - .toCompileTo(''); - - expectTemplate('{{__proto__}}') - .withInput({}) - .toCompileTo(''); - - expectTemplate('{{lookup this "constructor"}}') - .withInput({}) - .toCompileTo(''); - - expectTemplate('{{lookup this "__defineGetter__"}}') - .withInput({}) - .toCompileTo(''); - - expectTemplate('{{lookup this "__defineSetter__"}}') - .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('{{lookup this "__lookupGetter__"}}') - .withInput({}) - .toCompileTo(''); + TestClass.prototype.aProperty = 'propertyValue'; + TestClass.prototype.aMethod = function() { + return 'returnValue'; + }; - expectTemplate('{{lookup this "__proto__"}}') - .withInput({}) - .toCompileTo(''); + afterEach(function() { + sinon.restore(); }); - describe('GH-1631: disallow access to prototype functions', function() { - function TestClass() {} + describe('control access to prototype methods via "allowedProtoMethods"', function() { + checkProtoMethodAccess({}); + + describe('in compat mode', function() { + checkProtoMethodAccess({ compat: true }); + }); - TestClass.prototype.aProperty = 'propertyValue'; - TestClass.prototype.aMethod = function() { - return 'returnValue'; - }; + function checkProtoMethodAccess(compileOptions) { + it('should be prohibited by default and log a warning', function() { + var spy = sinon.spy(console, 'error'); - 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 }); + }); + + 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 explicitly prohibited by default, which disables the warning', function() { + var spy = sinon.spy(console, 'error'); - describe('control access to prototype non-methods via "allowedProtoProperties"', function() { - it('should be prohibited by default', function() { expectTemplate('{{aProperty}}') .withInput(new TestClass()) + .withCompileOptions(compileOptions) + .withRuntimeOptions({ + allowProtoPropertiesByDefault: false + }) .toCompileTo(''); + + expect(spy.callCount).to.equal(0); }); - it('can be turned on', function() { + 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, } ); } From 575d8772e2ccf05da235c596dd3405ae74194e1b Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Fri, 10 Jan 2020 17:05:23 +0100 Subject: [PATCH 02/12] fix: use "logger" instead of console.error ... to be graceful with older browser without "console" --- lib/handlebars/internal/proto-access.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/handlebars/internal/proto-access.js b/lib/handlebars/internal/proto-access.js index 59bf1444..a10f5124 100644 --- a/lib/handlebars/internal/proto-access.js +++ b/lib/handlebars/internal/proto-access.js @@ -1,4 +1,5 @@ import { createNewLookupObject } from './create-new-lookup-object'; +import * as logger from '../logger'; export function createProtoAccessControl(runtimeOptions) { let defaultMethodWhiteList = Object.create(null); @@ -44,9 +45,9 @@ function checkWhiteList(protoAccessControlForType, propertyName) { if (protoAccessControlForType.defaultValue !== undefined) { return protoAccessControlForType.defaultValue; } - // eslint-disable-next-line no-console - console.error( + 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 http://localhost:8080/api-reference/runtime-options.html#options-to-control-prototype-access for details` From 1f0834b1a2937150923f9de849b9612bd1969d11 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Fri, 10 Jan 2020 17:23:31 +0100 Subject: [PATCH 03/12] Update release notes --- release-notes.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/release-notes.md b/release-notes.md index d75b3202..ac4fc504 100644 --- a/release-notes.md +++ b/release-notes.md @@ -2,7 +2,22 @@ ## Development -[Commits](https://github.com/wycats/handlebars.js/compare/v4.6.0...master) +[Commits](https://github.com/wycats/handlebars.js/compare/v4.7.0...master) + +## 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 From 0d5c807017f8ba6c6d947f9d6852033c8faa2e49 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Fri, 10 Jan 2020 17:24:06 +0100 Subject: [PATCH 04/12] v4.7.0 --- components/bower.json | 2 +- components/handlebars.js.nuspec | 2 +- components/package.json | 2 +- lib/handlebars/base.js | 2 +- package.json | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/bower.json b/components/bower.json index c4501c0f..5bb095bc 100644 --- a/components/bower.json +++ b/components/bower.json @@ -1,6 +1,6 @@ { "name": "handlebars", - "version": "4.6.0", + "version": "4.7.0", "main": "handlebars.js", "license": "MIT", "dependencies": {} diff --git a/components/handlebars.js.nuspec b/components/handlebars.js.nuspec index 34886aea..91f7baf8 100644 --- a/components/handlebars.js.nuspec +++ b/components/handlebars.js.nuspec @@ -2,7 +2,7 @@ handlebars.js - 4.6.0 + 4.7.0 handlebars.js Authors https://github.com/wycats/handlebars.js/blob/master/LICENSE https://github.com/wycats/handlebars.js/ diff --git a/components/package.json b/components/package.json index 8e66182b..d040cda4 100644 --- a/components/package.json +++ b/components/package.json @@ -1,6 +1,6 @@ { "name": "handlebars", - "version": "4.6.0", + "version": "4.7.0", "license": "MIT", "jspm": { "main": "handlebars", diff --git a/lib/handlebars/base.js b/lib/handlebars/base.js index 88ba0ddc..a80e0dde 100644 --- a/lib/handlebars/base.js +++ b/lib/handlebars/base.js @@ -4,7 +4,7 @@ import { registerDefaultHelpers } from './helpers'; import { registerDefaultDecorators } from './decorators'; import logger from './logger'; -export const VERSION = '4.6.0'; +export const VERSION = '4.7.0'; export const COMPILER_REVISION = 8; export const LAST_COMPATIBLE_COMPILER_REVISION = 7; diff --git a/package.json b/package.json index dadcbb54..62e64050 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "handlebars", "barename": "handlebars", - "version": "4.6.0", + "version": "4.7.0", "description": "Handlebars provides the power necessary to let you build semantic templates effectively with no frustration", "homepage": "http://www.handlebarsjs.com/", "keywords": [ From 3c1e2521694583bc1d8bade1ed5b162f5bfb065a Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Sun, 12 Jan 2020 13:06:56 +0100 Subject: [PATCH 05/12] fix: log error for illegal property access only once per property --- lib/handlebars/base.js | 8 +++++++ lib/handlebars/internal/proto-access.js | 28 ++++++++++++++++++------- spec/security.js | 21 +++++++++++++++++++ 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/lib/handlebars/base.js b/lib/handlebars/base.js index a80e0dde..6e58ad8b 100644 --- a/lib/handlebars/base.js +++ b/lib/handlebars/base.js @@ -3,6 +3,7 @@ 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.7.0'; export const COMPILER_REVISION = 8; @@ -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/proto-access.js b/lib/handlebars/internal/proto-access.js index a10f5124..7bf9553f 100644 --- a/lib/handlebars/internal/proto-access.js +++ b/lib/handlebars/internal/proto-access.js @@ -1,6 +1,8 @@ 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; @@ -45,12 +47,24 @@ function checkWhiteList(protoAccessControlForType, propertyName) { if (protoAccessControlForType.defaultValue !== undefined) { return protoAccessControlForType.defaultValue; } - // eslint-disable-next-line no-console - 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 http://localhost:8080/api-reference/runtime-options.html#options-to-control-prototype-access for details` - ); + 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 http://localhost:8080/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/spec/security.js b/spec/security.js index bf0be148..1b345f0c 100644 --- a/spec/security.js +++ b/spec/security.js @@ -190,6 +190,10 @@ describe('security issues', function() { return 'returnValue'; }; + beforeEach(function() { + handlebarsEnv.resetLoggedPropertyAccesses(); + }); + afterEach(function() { sinon.restore(); }); @@ -214,6 +218,23 @@ describe('security issues', function() { expect(spy.args[0][0]).to.match(/Handlebars: Access has been denied/); }); + it('should only log the warning once', function() { + var spy = sinon.spy(console, 'error'); + + expectTemplate('{{aMethod}}') + .withInput(new TestClass()) + .withCompileOptions(compileOptions) + .toCompileTo(''); + + 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, which disables the warning', function() { var spy = sinon.spy(console, 'error'); From f152dfc89204e8c117605d602dac4fdc174ddcd9 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Sun, 12 Jan 2020 13:09:19 +0100 Subject: [PATCH 06/12] fix: fix log output in case of illegal property access - fix link url to handlebarsjs.com --- lib/handlebars/internal/proto-access.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/handlebars/internal/proto-access.js b/lib/handlebars/internal/proto-access.js index 7bf9553f..a8c5394d 100644 --- a/lib/handlebars/internal/proto-access.js +++ b/lib/handlebars/internal/proto-access.js @@ -58,7 +58,7 @@ function logUnexpecedPropertyAccessOnce(propertyName) { '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 http://localhost:8080/api-reference/runtime-options.html#options-to-control-prototype-access for details` + `See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details` ); } } From 4cddfe7017c28235ccad98f3434deb3725258da8 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Sun, 12 Jan 2020 13:20:37 +0100 Subject: [PATCH 07/12] Update release notes --- release-notes.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/release-notes.md b/release-notes.md index ac4fc504..8e391a77 100644 --- a/release-notes.md +++ b/release-notes.md @@ -2,7 +2,20 @@ ## Development -[Commits](https://github.com/wycats/handlebars.js/compare/v4.7.0...master) +[Commits](https://github.com/wycats/handlebars.js/compare/v4.7.1...master) + +## 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 From 14ba3d0c43d75bcfcdbfb7c95c9fac99d88a17c8 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Sun, 12 Jan 2020 13:21:08 +0100 Subject: [PATCH 08/12] v4.7.1 --- components/bower.json | 2 +- components/handlebars.js.nuspec | 2 +- components/package.json | 2 +- lib/handlebars/base.js | 2 +- package.json | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/bower.json b/components/bower.json index 5bb095bc..ccebdef1 100644 --- a/components/bower.json +++ b/components/bower.json @@ -1,6 +1,6 @@ { "name": "handlebars", - "version": "4.7.0", + "version": "4.7.1", "main": "handlebars.js", "license": "MIT", "dependencies": {} diff --git a/components/handlebars.js.nuspec b/components/handlebars.js.nuspec index 91f7baf8..44747f7f 100644 --- a/components/handlebars.js.nuspec +++ b/components/handlebars.js.nuspec @@ -2,7 +2,7 @@ handlebars.js - 4.7.0 + 4.7.1 handlebars.js Authors https://github.com/wycats/handlebars.js/blob/master/LICENSE https://github.com/wycats/handlebars.js/ diff --git a/components/package.json b/components/package.json index d040cda4..c296a32c 100644 --- a/components/package.json +++ b/components/package.json @@ -1,6 +1,6 @@ { "name": "handlebars", - "version": "4.7.0", + "version": "4.7.1", "license": "MIT", "jspm": { "main": "handlebars", diff --git a/lib/handlebars/base.js b/lib/handlebars/base.js index 6e58ad8b..2aa3a62f 100644 --- a/lib/handlebars/base.js +++ b/lib/handlebars/base.js @@ -5,7 +5,7 @@ import { registerDefaultDecorators } from './decorators'; import logger from './logger'; import { resetLoggedProperties } from './internal/proto-access'; -export const VERSION = '4.7.0'; +export const VERSION = '4.7.1'; export const COMPILER_REVISION = 8; export const LAST_COMPATIBLE_COMPILER_REVISION = 7; diff --git a/package.json b/package.json index 62e64050..5c289a2c 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "handlebars", "barename": "handlebars", - "version": "4.7.0", + "version": "4.7.1", "description": "Handlebars provides the power necessary to let you build semantic templates effectively with no frustration", "homepage": "http://www.handlebarsjs.com/", "keywords": [ From 9d5aa363cf3031b586e9945cf990e178f5b370db Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Mon, 13 Jan 2020 21:39:01 +0100 Subject: [PATCH 09/12] fix: don't wrap helpers that are not functions - helpers should always be a function, but in #1639 one seems to be undefined. This was not a problem before 4.6 because helpers weren't wrapped then. Now, we must take care only to wrap helpers (when adding the "lookupProperty" function to the options), if they are really functions. --- lib/handlebars/internal/wrapHelper.js | 5 +++++ spec/regressions.js | 9 +++++++++ 2 files changed, 14 insertions(+) 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/spec/regressions.js b/spec/regressions.js index 3a84dd7d..f6147ad6 100644 --- a/spec/regressions.js +++ b/spec/regressions.js @@ -518,4 +518,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'); + }); + }); }); From a4fd391ba1c9faa1004e879f314beb80c3afe0b6 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Mon, 13 Jan 2020 21:47:51 +0100 Subject: [PATCH 10/12] chore: execute saucelabs-task only if access-key exists - up to now, the existance of the SAUCE_USERNAME was checked but this variable is even present in pull-requests from other repos. This means that builds fail, because the access key is not there. This change looks for SAUCE_ACCESS_KEY instead, which is a secure variable, only present in build originating from the handlebars.js repo. --- Gruntfile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gruntfile.js b/Gruntfile.js index 65e0b8f3..528c0af8 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -267,7 +267,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', []); From f0c6c4cc1f9a91371535ad6affe09dfc1880dd9e Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Mon, 13 Jan 2020 21:52:50 +0100 Subject: [PATCH 11/12] Update release notes --- release-notes.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/release-notes.md b/release-notes.md index 8e391a77..09ecc2a0 100644 --- a/release-notes.md +++ b/release-notes.md @@ -2,7 +2,23 @@ ## Development -[Commits](https://github.com/wycats/handlebars.js/compare/v4.7.1...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 From 586e672c8bba7db787bc9bfe9a9fde4ec98d5b4f Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Mon, 13 Jan 2020 21:53:14 +0100 Subject: [PATCH 12/12] v4.7.2 --- components/bower.json | 2 +- components/handlebars.js.nuspec | 2 +- components/package.json | 2 +- lib/handlebars/base.js | 2 +- package.json | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/bower.json b/components/bower.json index ccebdef1..f62ac8ca 100644 --- a/components/bower.json +++ b/components/bower.json @@ -1,6 +1,6 @@ { "name": "handlebars", - "version": "4.7.1", + "version": "4.7.2", "main": "handlebars.js", "license": "MIT", "dependencies": {} diff --git a/components/handlebars.js.nuspec b/components/handlebars.js.nuspec index 44747f7f..078ea869 100644 --- a/components/handlebars.js.nuspec +++ b/components/handlebars.js.nuspec @@ -2,7 +2,7 @@ handlebars.js - 4.7.1 + 4.7.2 handlebars.js Authors https://github.com/wycats/handlebars.js/blob/master/LICENSE https://github.com/wycats/handlebars.js/ diff --git a/components/package.json b/components/package.json index c296a32c..0963613b 100644 --- a/components/package.json +++ b/components/package.json @@ -1,6 +1,6 @@ { "name": "handlebars", - "version": "4.7.1", + "version": "4.7.2", "license": "MIT", "jspm": { "main": "handlebars", diff --git a/lib/handlebars/base.js b/lib/handlebars/base.js index 2aa3a62f..40f2fc56 100644 --- a/lib/handlebars/base.js +++ b/lib/handlebars/base.js @@ -5,7 +5,7 @@ import { registerDefaultDecorators } from './decorators'; import logger from './logger'; import { resetLoggedProperties } from './internal/proto-access'; -export const VERSION = '4.7.1'; +export const VERSION = '4.7.2'; export const COMPILER_REVISION = 8; export const LAST_COMPATIBLE_COMPILER_REVISION = 7; diff --git a/package.json b/package.json index 5c289a2c..8b6658e8 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "handlebars", "barename": "handlebars", - "version": "4.7.1", + "version": "4.7.2", "description": "Handlebars provides the power necessary to let you build semantic templates effectively with no frustration", "homepage": "http://www.handlebarsjs.com/", "keywords": [