From d54137810a49939fd2ad01a91a34e182ece4528e Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Wed, 13 Nov 2019 21:41:36 +0100 Subject: [PATCH] fix: use String(field) in lookup when checking for "constructor" closes #1603 --- lib/handlebars/helpers/lookup.js | 2 +- spec/security.js | 28 +++++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lib/handlebars/helpers/lookup.js b/lib/handlebars/helpers/lookup.js index dcd585178..0654cc393 100644 --- a/lib/handlebars/helpers/lookup.js +++ b/lib/handlebars/helpers/lookup.js @@ -3,7 +3,7 @@ export default function(instance) { if (!obj) { return obj; } - if (field === 'constructor' && !obj.propertyIsEnumerable(field)) { + if (String(field) === 'constructor' && !obj.propertyIsEnumerable(field)) { return undefined; } return obj[field]; diff --git a/spec/security.js b/spec/security.js index c50bda6d0..80b8e23c2 100644 --- a/spec/security.js +++ b/spec/security.js @@ -1,11 +1,26 @@ 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}}', {}, ''); - shouldCompileTo('{{lookup (lookup this "constructor") "name"}}', {}, ''); + expectTemplate('{{lookup (lookup this "constructor") "name"}}') + .withInput({}) + .toCompileTo(''); + + expectTemplate('{{constructor.name}}') + .withInput({}) + .toCompileTo(''); }); - it('should allow the "constructor" property to be accessed if it is enumerable', function() { + it('GH-1603: should not allow constructors to be accessed (lookup via toString)', function() { + expectTemplate('{{lookup (lookup this (list "constructor")) "name"}}') + .withInput({}) + .withHelper('list', function(element) { + return [element]; + }) + .toCompileTo(''); + }); + + + 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'); @@ -14,6 +29,13 @@ describe('security issues', function() { }}, 'here we go'); }); + it('should allow the "constructor" property to be accessed if it is enumerable', function() { + shouldCompileTo('{{lookup (lookup this "constructor") "name"}}', {'constructor': { + 'name': 'here we go' + }}, 'here we go'); + }); + + it('should allow prototype properties that are not constructors', function() { function TestClass() { }