feat: access control to prototype properties via whitelist

Disallow access to prototype properties and methods by default.
Access to properties is always checked via
`Object.prototype.hasOwnProperty.call(parent, propertyName)`.

New runtime options:
- **allowedProtoMethods**: a string-to-boolean map of property-names that are allowed if they are methods of the parent object.
- **allowedProtoProperties**: a string-to-boolean map of property-names that are allowed if they are properties but not methods of the parent object.

```js
const template = handlebars.compile('{{aString.trim}}')
const result = template({ aString: '  abc  ' })
// result is empty, because trim is defined at String prototype
```

```js
const template = handlebars.compile('{{aString.trim}}')
const result = template({ aString: '  abc  ' }, {
  allowedProtoMethods: {
    trim: true
  }
})
// result = 'abc'
```

Implementation details: The method now "container.lookupProperty"
handles the prototype-checks and the white-lists. It is used in
- JavaScriptCompiler#nameLookup
- The "lookup"-helper (passed to all helpers as "options.lookupProperty")
- The "lookup" function at the container, which is used for recursive lookups in "compat" mode

Compatibility:
- **Old precompiled templates work with new runtimes**: The "options.lookupPropery"-function is passed to the helper by a wrapper, not by the compiled templated.
- **New templates work with old runtimes**: The template contains a function that is used as fallback if the "lookupProperty"-function cannot be found at the container. However, the runtime-options "allowedProtoProperties" and "allowedProtoMethods" only work with the newest runtime.

BREAKING CHANGE:
- access to prototype properties is forbidden completely by default
This commit is contained in:
Nils Knappmeier
2019-12-16 21:44:55 +01:00
committed by Nils Knappmeier
parent 164b7ff0de
commit d03b6ecfc4
12 changed files with 354 additions and 73 deletions
+1
View File
@@ -275,6 +275,7 @@ describe('blocks', function() {
'Goodbye cruel OMG!'
);
});
it('block with deep recursive pathed lookup', function() {
var string =
'{{#outer}}Goodbye {{#inner}}cruel {{omg.yes}}{{/inner}}{{/outer}}';
+11
View File
@@ -1328,4 +1328,15 @@ describe('helpers', function() {
);
});
});
describe('the lookupProperty-option', function() {
it('should be passed to custom helpers', function() {
expectTemplate('{{testHelper}}')
.withHelper('testHelper', function testHelper(options) {
return options.lookupProperty(this, 'testProperty');
})
.withInput({ testProperty: 'abc' })
.toCompileTo('abc');
});
});
});
+25
View File
@@ -81,4 +81,29 @@ describe('javascript-compiler api', function() {
shouldCompileTo('{{foo}}', { foo: 'food' }, 'food_foo');
});
});
describe('#isValidJavaScriptVariableName', function() {
// It is there and accessible and could be used by someone. That's why we don't remove it
// it 4.x. But if we keep it, we add a test
// This test should not encourage you to use the function. It is not needed any more
// and might be removed in 5.0
['test', 'abc123', 'abc_123'].forEach(function(validVariableName) {
it("should return true for '" + validVariableName + "'", function() {
expect(
handlebarsEnv.JavaScriptCompiler.isValidJavaScriptVariableName(
validVariableName
)
).to.be.true();
});
});
[('123test', 'abc()', 'abc.cde')].forEach(function(invalidVariableName) {
it("should return true for '" + invalidVariableName + "'", function() {
expect(
handlebarsEnv.JavaScriptCompiler.isValidJavaScriptVariableName(
invalidVariableName
)
).to.be.false();
});
});
});
});
+48 -6
View File
@@ -390,7 +390,7 @@ describe('Regressions', function() {
it('should compile and execute templates', function() {
var newHandlebarsInstance = Handlebars.create();
registerTemplate(newHandlebarsInstance);
registerTemplate(newHandlebarsInstance, compiledTemplateVersion7());
newHandlebarsInstance.registerHelper('loud', function(value) {
return value.toUpperCase();
});
@@ -405,7 +405,7 @@ describe('Regressions', function() {
shouldThrow(
function() {
registerTemplate(newHandlebarsInstance);
registerTemplate(newHandlebarsInstance, compiledTemplateVersion7());
newHandlebarsInstance.templates['test.hbs']({});
},
Handlebars.Exception,
@@ -413,11 +413,31 @@ describe('Regressions', function() {
);
});
// This is a only slightly modified precompiled templated from compiled with 4.2.1
function registerTemplate(Handlebars) {
it('should pass "options.lookupProperty" to "lookup"-helper, even with old templates', function() {
var newHandlebarsInstance = Handlebars.create();
registerTemplate(
newHandlebarsInstance,
compiledTemplateVersion7_usingLookupHelper()
);
newHandlebarsInstance.templates['test.hbs']({});
expect(
newHandlebarsInstance.templates['test.hbs']({
property: 'a',
test: { a: 'b' }
})
).to.equal('b');
});
function registerTemplate(Handlebars, compileTemplate) {
var template = Handlebars.template,
templates = (Handlebars.templates = Handlebars.templates || {});
templates['test.hbs'] = template({
templates['test.hbs'] = template(compileTemplate);
}
function compiledTemplateVersion7() {
return {
compiler: [7, '>= 4.0.0'],
main: function(container, depth0, helpers, partials, data) {
return (
@@ -435,7 +455,29 @@ describe('Regressions', function() {
);
},
useData: true
});
};
}
function compiledTemplateVersion7_usingLookupHelper() {
// This is the compiled version of "{{lookup test property}}"
return {
compiler: [7, '>= 4.0.0'],
main: function(container, depth0, helpers, partials, data) {
return container.escapeExpression(
helpers.lookup.call(
depth0 != null ? depth0 : container.nullContext || {},
depth0 != null ? depth0.test : depth0,
depth0 != null ? depth0.property : depth0,
{
name: 'lookup',
hash: {},
data: data
}
)
);
},
useData: true
};
}
});
+153 -29
View File
@@ -19,7 +19,7 @@ describe('security issues', function() {
.toCompileTo('');
});
it('should allow the "constructor" property to be accessed if it is enumerable', function() {
it('should allow the "constructor" property to be accessed if it is an "ownProperty"', function() {
shouldCompileTo(
'{{constructor.name}}',
{
@@ -40,7 +40,7 @@ describe('security issues', function() {
);
});
it('should allow the "constructor" property to be accessed if it is enumerable', function() {
it('should allow the "constructor" property to be accessed if it is an "own property"', function() {
shouldCompileTo(
'{{lookup (lookup this "constructor") "name"}}',
{
@@ -51,27 +51,6 @@ describe('security issues', function() {
'here we go'
);
});
it('should allow prototype properties that are not constructors', function() {
function TestClass() {}
Object.defineProperty(TestClass.prototype, 'abc', {
get: function() {
return 'xyz';
}
});
shouldCompileTo(
'{{#with this as |obj|}}{{obj.abc}}{{/with}}',
new TestClass(),
'xyz'
);
shouldCompileTo(
'{{#with this as |obj|}}{{lookup obj "abc"}}{{/with}}',
new TestClass(),
'xyz'
);
});
});
describe('GH-1558: Prevent explicit call of helperMissing-helpers', function() {
@@ -171,38 +150,183 @@ describe('security issues', function() {
});
describe('GH-1595', function() {
it('properties, that are required to be enumerable', 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 "constructor"}}')
expectTemplate('{{lookup this "constructor"}}')
.withInput({})
.toCompileTo('');
expectTemplate('{{lookup "__defineGetter__"}}')
expectTemplate('{{lookup this "__defineGetter__"}}')
.withInput({})
.toCompileTo('');
expectTemplate('{{lookup "__defineSetter__"}}')
expectTemplate('{{lookup this "__defineSetter__"}}')
.withInput({})
.toCompileTo('');
expectTemplate('{{lookup "__lookupGetter__"}}')
expectTemplate('{{lookup this "__lookupGetter__"}}')
.withInput({})
.toCompileTo('');
expectTemplate('{{lookup "__proto__"}}')
expectTemplate('{{lookup this "__proto__"}}')
.withInput({})
.toCompileTo('');
});
describe('GH-1631: disallow access to prototype functions', function() {
function TestClass() {}
TestClass.prototype.aProperty = 'propertyValue';
TestClass.prototype.aMethod = function() {
return 'returnValue';
};
describe('control access to prototype methods via "allowedProtoMethods"', function() {
it('should be prohibited by default', function() {
expectTemplate('{{aMethod}}')
.withInput(new TestClass())
.toCompileTo('');
});
it('can be allowed', function() {
expectTemplate('{{aMethod}}')
.withInput(new TestClass())
.withRuntimeOptions({
allowedProtoMethods: {
aMethod: true
}
})
.toCompileTo('returnValue');
});
it('should be prohibited by default (in "compat" mode)', function() {
expectTemplate('{{aMethod}}')
.withInput(new TestClass())
.withCompileOptions({ compat: true })
.toCompileTo('');
});
it('can be allowed (in "compat" mode)', function() {
expectTemplate('{{aMethod}}')
.withInput(new TestClass())
.withCompileOptions({ compat: true })
.withRuntimeOptions({
allowedProtoMethods: {
aMethod: true
}
})
.toCompileTo('returnValue');
});
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"', function() {
it('should be prohibited by default', function() {
expectTemplate('{{aProperty}}')
.withInput(new TestClass())
.toCompileTo('');
});
it('can be turned on', function() {
expectTemplate('{{aProperty}}')
.withInput(new TestClass())
.withRuntimeOptions({
allowedProtoProperties: {
aProperty: true
}
})
.toCompileTo('propertyValue');
});
it('should be prohibited by default (in "compat" mode)', function() {
expectTemplate('{{aProperty}}')
.withInput(new TestClass())
.withCompileOptions({ compat: true })
.toCompileTo('');
});
it('can be turned on (in "compat" mode)', function() {
expectTemplate('{{aProperty}}')
.withInput(new TestClass())
.withCompileOptions({ compat: true })
.withRuntimeOptions({
allowedProtoProperties: {
aProperty: true
}
})
.toCompileTo('propertyValue');
});
});
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();
});
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');
});
});
});
});
});
function wrapToAdjustContainer(precompiledTemplateFunction) {
return function templateFunctionWrapper(container /*, more args */) {
delete container.lookupProperty;
return precompiledTemplateFunction.apply(this, arguments);
};
}