Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport security fixes to 3.x branch #1532

Merged
merged 5 commits into from Jun 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions 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
Expand Down
37 changes: 37 additions & 0 deletions 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}"
8 changes: 7 additions & 1 deletion lib/handlebars/base.js
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
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
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
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
@@ -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
@@ -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
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