From de6ded486f84cf685e37e1bd719aad7211176241 Mon Sep 17 00:00:00 2001 From: Matt Olson Date: Fri, 28 Jun 2019 23:12:30 -0700 Subject: [PATCH 1/5] Backport security fixes to 3.x branch --- lib/handlebars/base.js | 8 ++++- .../compiler/javascript-compiler.js | 3 ++ spec/security.js | 33 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 spec/security.js diff --git a/lib/handlebars/base.js b/lib/handlebars/base.js index cfe1e917c..5342f294f 100644 --- a/lib/handlebars/base.js +++ b/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]; }); } diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index 883066165..f070a39f9 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/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 { diff --git a/spec/security.js b/spec/security.js new file mode 100644 index 000000000..5af5334eb --- /dev/null +++ b/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'); + }); + }); +}); From 47adcda48530ab1504b8019fe17eaedd4f4c943f Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Sun, 14 May 2017 23:11:43 +0200 Subject: [PATCH 2/5] Fix build on Windows - Handle path-separators properly. Use "path.sep" instead of "/". Or use "require.resolve()" if possible - Use "execFile" instead of "exec" to run the Handlebars executable. This prevents problems due to (missing) shell escaping. - Use explicit call to "node" in order to run the executable on Windows. - Add "appveyor"-CI in order to run regular tests on Windows. --- README.markdown | 1 + appveyor.yml | 38 ++++++++++++++++++++++++++++++++++++++ spec/env/browser.js | 8 +++++++- spec/env/runner.js | 2 +- tasks/test.js | 13 +++++++++++-- 5 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 appveyor.yml diff --git a/README.markdown b/README.markdown index d6aa6bd5c..9e62181a8 100644 --- a/README.markdown +++ b/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 diff --git a/appveyor.yml b/appveyor.yml new file mode 100644 index 000000000..b67fb4ca5 --- /dev/null +++ b/appveyor.yml @@ -0,0 +1,38 @@ +# Test against these versions of Node.js +environment: + matrix: + - nodejs_version: "4" + - nodejs_version: "5" + +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}" \ No newline at end of file diff --git a/spec/env/browser.js b/spec/env/browser.js index c9414d402..76a968595 100644 --- a/spec/env/browser.js +++ b/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, diff --git a/spec/env/runner.js b/spec/env/runner.js index 10a73d848..0eced7072 100644 --- a/spec/env/runner.js +++ b/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() { diff --git a/tasks/test.js b/tasks/test.js index ad8a911d3..3ddbf683f 100644 --- a/tasks/test.js +++ b/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; } From 420ac171a01b8777ebce0a777221754fcc72a5a8 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Thu, 7 Feb 2019 10:14:44 +0100 Subject: [PATCH 3/5] test: run appveyor tests in Node 10 --- appveyor.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index b67fb4ca5..563aaf93c 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,8 +1,7 @@ # Test against these versions of Node.js environment: matrix: - - nodejs_version: "4" - - nodejs_version: "5" + - nodejs_version: "10" platform: - x64 From 7820b207e123babd0bda0b4871790f2ea6b36b01 Mon Sep 17 00:00:00 2001 From: Matt Olson Date: Fri, 28 Jun 2019 23:52:09 -0700 Subject: [PATCH 4/5] Use istanbul/lib/cli.js instead of node_modules/.bin/istanbul Due to the way, "bin"-files are distributed into the node_modules/.bin directory on Windows, the task "test:cov" did not work on Windows. This commit uses the node-script directly. --- tasks/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/test.js b/tasks/test.js index 3ddbf683f..b5c88ad09 100644 --- a/tasks/test.js +++ b/tasks/test.js @@ -41,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'); From 7c3944015d30a4348ae66ec1736b752cd864d5c1 Mon Sep 17 00:00:00 2001 From: Matt Olson Date: Sat, 29 Jun 2019 09:38:30 -0700 Subject: [PATCH 5/5] Fix Travis by updating git tag retrieval --- tasks/util/git.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/util/git.js b/tasks/util/git.js index a6c9ec1dc..abb7723e8 100644 --- a/tasks/util/git.js +++ b/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); }