Skip to content

Commit

Permalink
fix: disallow access to the constructor in templates to prevent RCE
Browse files Browse the repository at this point in the history
This commit fixes a Remote Code Execution (RCE) reported by
npm-security. Access to non-enumerable "constructor"-properties
is now prohibited by the compiled template-code, because this
the first step on the way to creating and execution arbitrary
JavaScript code.
The vulnerability affects systems where an attacker is allowed to
inject templates into the Handlebars setup.
Further details of the attack may be disclosed by npm-security.

Closes #1267
Closes #1495
  • Loading branch information
nknapp committed Feb 7, 2019
1 parent bacd473 commit edc6220
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 0 deletions.
3 changes: 3 additions & 0 deletions lib/handlebars/compiler/javascript-compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ JavaScriptCompiler.prototype = {
// PUBLIC API: You can override these methods in a subclass to provide
// alternative compiled forms for name lookup and buffering semantics
nameLookup: function(parent, name/* , type*/) {
if (name === 'constructor') {
return ['(', parent, '.propertyIsEnumerable(\'constructor\') ? ', parent, '.constructor : undefined', ')'];
}
if (JavaScriptCompiler.isValidJavaScriptVariableName(name)) {
return [parent, '.', name];
} else {
Expand Down
23 changes: 23 additions & 0 deletions spec/security.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
describe('security issues', function() {
describe('GH-1495: Prevent Remote Code Execution via constructor', function() {
it('should not allow constructors to be accessed', function() {
shouldCompileTo('{{constructor.name}}', {}, '');
});

it('should allow the "constructor" property to be accessed if it is enumerable', function() {
shouldCompileTo('{{constructor.name}}', {'constructor': {
'name': 'here we go'
}}, 'here we go');
});

it('should allow prototype properties that are not constructors', function() {
class TestClass {
get abc() {
return 'xyz';
}
}
shouldCompileTo('{{#with this as |obj|}}{{obj.abc}}{{/with}}',
new TestClass(), 'xyz');
});
});
});

2 comments on commit edc6220

@MCoop1963
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated security risk correction

@nknapp
Copy link
Collaborator Author

@nknapp nknapp commented on edc6220 Jul 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MCoop1963 I don't understand your last comment. Could you elaborate?

Please sign in to comment.