Skip to content

Commit

Permalink
Merge pull request #1532 from mattolson/backport-security-fixes
Browse files Browse the repository at this point in the history
Backport security fixes to 3.x branch
  • Loading branch information
nknapp authored Jun 29, 2019
2 parents 7cf753b + 7c39440 commit 0d6d8c3
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 7 deletions.
1 change: 1 addition & 0 deletions README.markdown
Original file line number Diff line number Diff line change
@@ -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
Expand Down
37 changes: 37 additions & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -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}"
8 changes: 7 additions & 1 deletion lib/handlebars/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
});
}

Expand Down
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
8 changes: 7 additions & 1 deletion spec/env/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion spec/env/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
33 changes: 33 additions & 0 deletions spec/security.js
Original file line number Diff line number Diff line change
@@ -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');
});
});
});
15 changes: 12 additions & 3 deletions tasks/test.js
Original file line number Diff line number Diff line change
@@ -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;
}
Expand All @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion tasks/util/git.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 0d6d8c3

Please sign in to comment.