fix: use String(field) in lookup when checking for "constructor"

closes #1603
handlebars-lang · Nov 13, 2019 · d541378 · d541378
1 parent c2ac79c
commit d541378
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 4 deletions.
diff --git 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
@@ -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() {
             }