Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.1.x: Option to configure escaping in Handelbars.escapeExpression (escape "=") #1492

Closed
7 tasks
nknapp opened this issue Jan 2, 2019 · 1 comment
Closed
7 tasks

Comments

@nknapp
Copy link
Collaborator

nknapp commented Jan 2, 2019

This is a follow-up issue for #1454. It's goal is to discuss the implementation of a fix for security advisory 61 in the 3.x branch.

Some people have asked to fix the issue in 3.0.x (#1454). When this was done, other people rightfully claimed that this was a semver-violation (#1489). The fix was reverted in 3.0.6, but we still need an option
to resolve the problem.

The only way to comply to semver rules is to build a 3.1 version with an option to activate escaping of "=". @wycats also proposed to add a warning output if the option is not set.

The most simple way would be:

  • Add a function Handlebars.escapeEqualSign() that replaces escapeExpression in the Handlebars environment by a version that escapes the "="-character as well.
  • Print a warning when escapeExpression is called and the property is not set.

The drawback would be that the warning would only be printed in unit-tests and on the production site, which would invite attackers to exploit the vulnaribility.

Another way would be to:

  • Add an optional second argument (an options-object) to Handlebars.escapeExpression that activates escaping of "=".
  • Add a compiler option that adds the ensures that Handlebars.escapeExpression is always called with this option, when {{mustache}} expressions are found in the template.
  • Print a warning, if the compiler option is not set.
  • Print a warning, when escapeExpression is called for the first time without the option (no need to be overly verbose)
  • Optional: Add a confguration method Handlebars.failIfUnsafeEscape() that causes Handlebars.escapeExpression to throw an exception when called without the option. This could be used in builds to find all the places where the option is still missing.

This would allow developers to add the option to their precompile step and the warning would usually only be printed at build-time and not presented to the users (and potential XSS-hackers).

Either way, the version 3.1 would still be marked as insecure in the security-database.

Since I am not personally using this version anymore, I'd like to hear preferences or other ideas from people who need this change.

/cc @mattolson @courtneynguyen

@nknapp
Copy link
Collaborator Author

nknapp commented Apr 5, 2020

Closing this due to inactivity, please comment or submit a PR if you want to see this in the 3.x branch

@nknapp nknapp closed this as completed Apr 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant