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 000000000..f6bb4cb15 --- /dev/null +++ b/lib/handlebars/internal/proto-access.js @@ -0,0 +1,48 @@ +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; + } + return false; +} diff --git a/lib/handlebars/runtime.js b/lib/handlebars/runtime.js index 7d63e87b8..3dfcc499a 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( @@ -129,12 +131,8 @@ export function template(templateSpec, env) { 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 +235,7 @@ export function template(templateSpec, env) { ) ); } + main = executeDecorators( templateSpec.main, main, @@ -247,6 +246,7 @@ export function template(templateSpec, env) { ); return main(context, options); } + ret.isTop = true; ret._setup = function(options) { @@ -271,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 || @@ -284,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/spec/security.js b/spec/security.js index 7bf9d89a0..d2c55ab93 100644 --- a/spec/security.js +++ b/spec/security.js @@ -149,67 +149,66 @@ 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(''); - - expectTemplate('{{lookup this "__lookupGetter__"}}') - .withInput({}) - .toCompileTo(''); - - expectTemplate('{{lookup this "__proto__"}}') - .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() {} - describe('GH-1631: disallow access to prototype functions', function() { - function TestClass() {} + TestClass.prototype.aProperty = 'propertyValue'; + TestClass.prototype.aMethod = function() { + return 'returnValue'; + }; - TestClass.prototype.aProperty = 'propertyValue'; - TestClass.prototype.aMethod = function() { - return 'returnValue'; - }; + describe('control access to prototype methods via "allowedProtoMethods"', function() { + checkProtoMethodAccess({}); + + describe('in compat mode', function() { + checkProtoMethodAccess({ compat: true }); + }); - describe('control access to prototype methods via "allowedProtoMethods"', function() { + function checkProtoMethodAccess(compileOptions) { it('should be prohibited by default', function() { expectTemplate('{{aMethod}}') .withInput(new TestClass()) + .withCompileOptions(compileOptions) .toCompileTo(''); }); it('can be allowed', function() { expectTemplate('{{aMethod}}') .withInput(new TestClass()) + .withCompileOptions(compileOptions) .withRuntimeOptions({ allowedProtoMethods: { aMethod: true @@ -218,55 +217,69 @@ describe('security issues', function() { .toCompileTo('returnValue'); }); - it('should be prohibited by default (in "compat" mode)', function() { + it('can be turned on by default', function() { expectTemplate('{{aMethod}}') .withInput(new TestClass()) - .withCompileOptions({ compat: true }) - .toCompileTo(''); + .withCompileOptions(compileOptions) + .withRuntimeOptions({ + allowProtoMethodsByDefault: true + }) + .toCompileTo('returnValue'); }); - it('can be allowed (in "compat" mode)', function() { + it('can be turned off, if turned on by default', function() { expectTemplate('{{aMethod}}') .withInput(new TestClass()) - .withCompileOptions({ compat: true }) + .withCompileOptions(compileOptions) .withRuntimeOptions({ + allowProtoMethodsByDefault: true, allowedProtoMethods: { - aMethod: true + aMethod: 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'); - }); + 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'); - }); + 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() { + function checkProtoPropertyAccess(compileOptions) { it('should be prohibited by default', function() { expectTemplate('{{aProperty}}') .withInput(new TestClass()) + .withCompileOptions(compileOptions) .toCompileTo(''); }); it('can be turned on', function() { expectTemplate('{{aProperty}}') .withInput(new TestClass()) + .withCompileOptions(compileOptions) .withRuntimeOptions({ allowedProtoProperties: { aProperty: true @@ -275,50 +288,54 @@ describe('security issues', function() { .toCompileTo('propertyValue'); }); - it('should be prohibited by default (in "compat" mode)', function() { + it('can be turned on by default', function() { expectTemplate('{{aProperty}}') .withInput(new TestClass()) - .withCompileOptions({ compat: true }) - .toCompileTo(''); + .withCompileOptions(compileOptions) + .withRuntimeOptions({ + allowProtoPropertiesByDefault: true + }) + .toCompileTo('propertyValue'); }); - 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 606741f5a..1fa83680d 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 b081ae4fb..7a34b7eba 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, } ); }