Merge pull request #1532 from mattolson/backport-security-fixes

Backport security fixes to 3.x branch

Author: nknapp

README.markdown

@@ -1,4 +1,5 @@
 [![Travis Build Status](https://img.shields.io/travis/wycats/handlebars.js/master.svg)](https://travis-ci.org/wycats/handlebars.js)
+[![Appveyor Build Status](https://ci.appveyor.com/api/projects/status/github/wycats/handlebars.js?branch=master&svg=true)](https://ci.appveyor.com/project/wycats/handlebars-js)
 [![Selenium Test Status](https://saucelabs.com/buildstatus/handlebars)](https://saucelabs.com/u/handlebars)
 
 Handlebars.js

appveyor.yml

@@ -0,0 +1,37 @@
+# Test against these versions of Node.js
+environment:
+  matrix:
+    - nodejs_version: "10"
+
+platform:
+  - x64
+
+# Install scripts (runs after repo cloning)
+install:
+  # Get the latest stable version of Node.js
+  - ps: Install-Product node $env:nodejs_version $env:platform
+  # Clone submodules (mustache spec)
+  - cmd: git submodule update --init --recursive
+  # Install modules
+  - cmd: npm install
+  - cmd: npm install -g grunt-cli
+
+
+# Post-install test scripts
+test_script:
+  # Output useful info for debugging
+  - cmd: node --version
+  - cmd: npm --version
+  # Run tests
+  - cmd: grunt --stack travis
+
+# Don't actually build
+build: off
+
+on_failure:
+  - cmd: 7z a coverage.zip coverage
+  - cmd: appveyor PushArtifact coverage.zip
+
+
+# Set build version format here instead of in the admin panel
+version: "{build}"

lib/handlebars/base.js

@@ -212,7 +212,13 @@ function registerDefaultHelpers(instance) {
   });
 
   instance.registerHelper('lookup', function(obj, field) {
-    return obj && obj[field];
+    if (!obj) {
+      return obj;
+    }
+    if (field === 'constructor' && !obj.propertyIsEnumerable(field)) {
+      return undefined;
+    }
+    return obj[field];
   });
 }
 

lib/handlebars/compiler/javascript-compiler.js

@@ -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 {

spec/env/browser.js

@@ -4,7 +4,13 @@ var fs = require('fs'),
     vm = require('vm');
 
 global.Handlebars = 'no-conflict';
-vm.runInThisContext(fs.readFileSync(__dirname + '/../../dist/handlebars.js'), 'dist/handlebars.js');
+
+var filename = 'dist/handlebars.js';
+if (global.minimizedTest) {
+  filename = 'dist/handlebars.min.js';
+}
+var distHandlebars = fs.readFileSync(require.resolve(`../../${filename}`), 'utf-8');
+vm.runInThisContext(distHandlebars, filename);
 
 global.CompilerContext = {
   browser: true,

spec/env/runner.js

@@ -11,7 +11,7 @@ var files = [ testDir + '/basic.js' ];
 
 var files = fs.readdirSync(testDir)
       .filter(function(name) { return (/.*\.js$/).test(name); })
-      .map(function(name) { return testDir + '/' + name; });
+      .map(function(name) { return testDir + path.sep + name; });
 
 run('./node', function() {
   run('./browser', function() {

spec/security.js

@@ -0,0 +1,33 @@
+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"}}', {}, '');
+        });
+
+        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');
+            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() {
+            }
+
+            Object.defineProperty(TestClass.prototype, 'abc', {
+                get: function() {
+                    return 'xyz';
+                }
+            });
+
+            shouldCompileTo('{{#with this}}{{this.abc}}{{/with}}',
+                new TestClass(), 'xyz');
+            shouldCompileTo('{{#with this}}{{lookup this "abc"}}{{/with}}',
+                new TestClass(), 'xyz');
+        });
+    });
+});

tasks/test.js

@@ -1,11 +1,20 @@
 var childProcess = require('child_process'),
-    fs = require('fs');
+    fs = require('fs'),
+    os = require('os');
 
 module.exports = function(grunt) {
   grunt.registerTask('test:bin', function() {
     var done = this.async();
 
-    childProcess.exec('./bin/handlebars -a spec/artifacts/empty.handlebars', function(err, stdout) {
+    var cmd = './bin/handlebars';
+    var args = [ '-a', 'spec/artifacts/empty.handlebars' ];
+
+    // On Windows, the executable handlebars.js file cannot be run directly
+    if (os.platform() === 'win32') {
+      args.unshift(cmd);
+      cmd = process.argv[0];
+    }
+    childProcess.execFile(cmd, args, function(err, stdout) {
       if (err) {
         throw err;
       }
@@ -32,7 +41,7 @@ module.exports = function(grunt) {
   grunt.registerTask('test:cov', function() {
     var done = this.async();
 
-    var runner = childProcess.fork('node_modules/.bin/istanbul', ['cover', '--', './spec/env/runner.js'], {stdio: 'inherit'});
+    var runner = childProcess.fork('node_modules/istanbul/lib/cli.js', ['cover', '--source-map', '--', './spec/env/runner.js'], {stdio: 'inherit'});
     runner.on('close', function(code) {
       if (code != 0) {
         grunt.fatal(code + ' tests failed');

tasks/util/git.js

@@ -86,7 +86,7 @@ module.exports = {
     });
   },
   tagName: function(callback) {
-    childProcess.exec('git describe --tags', {}, function(err, stdout) {
+    childProcess.exec('git describe --tags --always', {}, function(err, stdout) {
       if (err) {
         throw new Error('git.tagName: ' + err.message);
       }