feat: access control to prototype properties via whitelist

[nknapp]

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.

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

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

closes #1631
closes #1628

[Gerrit0]

@nknapp what would be your recommendation for projects which make heavy use of templates with helper methods attached to the prototype? TypeDoc uses handlebars and passes one of many different model classes to a template that will then render it.

For now, I'm going to pin TypeDoc to an older version of Handlebars with support for prototype properties/methods, but this really isn't a long term solution since staying up to date is important for security issues.

[nknapp]

This change is the response to a security issue. The biggest one I have seen so far, at least if you are running Handlebars in a web-server. See http://mahmoudsec.blogspot.com/2019/04/handlebars-template-injection-and-rce.html for details.

My problem is: This issue has first appeared end of 2018. Nobody has thought about that vulnerability until then. When the principal got public, people started poking around and tried to circumvent my patches. I always tried minimal changes so far. And I have had, I think, three variants of the issue that again led to three new versions of handlebars, during the last year.

The actual proposal is, to use only registered helpers to implement logic in the templates. But I can see that this may be a lot of work for you.

There are two options:

• Revert, but this may leave servers open to possible attacks
• add another option that defines the default value of prototype prohibition.
  • make it opt-in (this will again leave servers open to possible attacks)
  • make it opt-out (this will require setting a runtime-option to establish the old behaviour)

Would it be ok for you to have an opt-out?

What I mean is an option: "allowProtoAccessByDefault" that is by default set to "false".

[Gerrit0]

An opt-out would be ideal for my use case. TypeDoc already runs arbitrary user code when rendering themes, so the mentioned attack is a non-issue. A new option or being able to set allowedProtoMethods to true would work great!

I have been meaning to attack the themes and clean them up, maybe it is time to move that issue up the priority list and remove the need for prototype access entirely. It would be a lot of work, but perhaps that time has come.

[nknapp]

see #1635

Please have a look and comment.

Implementing this was is not so much work now, because the general logic of runtime-options controlling this behaviour is already there.

[nknapp]

Handlebars 4.7.0 has been release with options to disable prototype restrictions:
https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access

[tmaiaroto]

Would JSON.parse(JSON.stringify(objWithGetter)) not have worked as a safe default behavior? When passing templates responses from ORMs (for example Sequelize ORM), you'll now need to get a plain object instead of being able to leverage its getters, eager loading, etc. In the case of Sequelize, it was a quick fix to call toJSON() but it sure was confusing to track this down and the minor version represented an unexpected break for something extremely common.

[nknapp]

@tmaiaroto do you mean: as safe default behaviour instead of test hasOwnProperty for each access? Or instead of the changes I made in 4.7?

[Gerrit0]

JSON.parse(JSON.stringify(obj)) is far from safe. JSON doesn't handle recursive data structures, while Handlebars does support them.

[tmaiaroto]

Yea, and I did run into that. Oh well, bummer.

[nknapp]

JSON.parse also does not support functions. It would also not fix the vulnerabilities to do stringify-parse on the input object because the prototype functions would still be there.

I actually would like to do the white listing differently in 5.0. It would probably make more sense to pass class constructors as a runtime option and allow prototype properties of exactly those classes.

[tmaiaroto]

Hmm, not in front of computer, but you sure the prototype functions would still be there? Because I was able to pass an object that way and it went through. So if it still carries a vulnerability, it's getting through the check.