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

Harden creation of child processes #55697

Merged
merged 17 commits into from
Mar 10, 2020
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,16 @@ module.exports = {
},
},

/**
* Harden specific rules
*/
{
files: ['test/harden/*.js'],
rules: {
'mocha/handle-done-callback': 'off', // TODO: Find a way to disable all mocha rules
},
},

/**
* APM overrides
*/
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@
"regenerator-runtime": "^0.13.3",
"regression": "2.0.1",
"request": "^2.88.0",
"require-in-the-middle": "^5.0.2",
"reselect": "^4.0.0",
"resize-observer-polyfill": "^1.5.0",
"rison-node": "1.0.2",
Expand Down Expand Up @@ -480,6 +481,7 @@
"strip-ansi": "^3.0.1",
"supertest": "^3.1.0",
"supertest-as-promised": "^4.0.2",
"tape": "^4.13.0",
mistic marked this conversation as resolved.
Show resolved Hide resolved
"tree-kill": "^1.2.2",
"typescript": "3.7.2",
"typings-tester": "^0.3.2",
Expand Down
41 changes: 41 additions & 0 deletions scripts/test_hardening.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

var execFileSync = require('child_process').execFileSync;
var path = require('path');
var syncGlob = require('glob').sync;
var program = require('commander');

program
.name('node scripts/test_hardening.js')
.arguments('[file...]')
.description(
'Run the tests in test/harden directory. If no files are provided, all files within the directory will be run.'
)
.action(function(globs) {
if (globs.length === 0) globs.push(path.join('test', 'harden', '*'));
globs.forEach(function(glob) {
syncGlob(glob).forEach(function(filename) {
if (path.basename(filename)[0] === '_') return;
console.log(process.argv[0], filename);
execFileSync(process.argv[0], [filename], { stdio: 'inherit' });
});
});
})
.parse(process.argv);
24 changes: 24 additions & 0 deletions src/setup_node_env/harden.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

var hook = require('require-in-the-middle');

hook(['child_process'], function(exports, name) {
return require(`./patches/${name}`)(exports); // eslint-disable-line import/no-dynamic-require
});
1 change: 1 addition & 0 deletions src/setup_node_env/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/

require('./harden'); // this require MUST be executed before any others
require('symbol-observable');
require('./root');
require('./node_version_validator');
Expand Down
76 changes: 76 additions & 0 deletions src/setup_node_env/patches/child_process.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

// Ensure, when spawning a new child process, that the `options` and the
// `options.env` object passed to the child process function doesn't inherit
// from `Object.prototype`. This protects against similar RCE vulnerabilities
// as described in CVE-2019-7609
module.exports = function(cp) {
watson marked this conversation as resolved.
Show resolved Hide resolved
// The `exec` function is currently just a wrapper around `execFile`. So for
// now there's no need to patch it. If this changes in the future, our tests
// will fail and we can comment out the line below.
watson marked this conversation as resolved.
Show resolved Hide resolved
//
// cp.exec = new Proxy(cp.exec, { apply: patchOptions() });

cp.execFile = new Proxy(cp.execFile, { apply: patchOptions(true) });
cp.fork = new Proxy(cp.fork, { apply: patchOptions(true) });
cp.spawn = new Proxy(cp.spawn, { apply: patchOptions(true) });
cp.execFileSync = new Proxy(cp.execFileSync, { apply: patchOptions(true) });
cp.execSync = new Proxy(cp.execSync, { apply: patchOptions() });
cp.spawnSync = new Proxy(cp.spawnSync, { apply: patchOptions(true) });

return cp;
};

function patchOptions(hasArgs) {
return function apply(target, thisArg, args) {
kobelb marked this conversation as resolved.
Show resolved Hide resolved
var pos = 1;
if (pos === args.length) {
// fn(arg1)
args[pos] = prototypelessSpawnOpts();
} else if (pos < args.length) {
if (hasArgs && (Array.isArray(args[pos]) || args[pos] == null)) {
// fn(arg1, args, ...)
pos++;
}

if (typeof args[pos] === 'object' && args[pos] !== null) {
// fn(arg1, {}, ...)
// fn(arg1, args, {}, ...)
args[pos] = prototypelessSpawnOpts(args[pos]);
} else if (args[pos] == null) {
// fn(arg1, null/undefined, ...)
// fn(arg1, args, null/undefined, ...)
args[pos] = prototypelessSpawnOpts();
} else if (typeof args[pos] === 'function') {
// fn(arg1, callback)
// fn(arg1, args, callback)
args.splice(pos, 0, prototypelessSpawnOpts());
}
}

return target.apply(thisArg, args);
};
}

function prototypelessSpawnOpts(obj) {
var prototypelessObj = Object.assign(Object.create(null), obj);
watson marked this conversation as resolved.
Show resolved Hide resolved
prototypelessObj.env = Object.assign(Object.create(null), prototypelessObj.env || process.env);
return prototypelessObj;
}
6 changes: 6 additions & 0 deletions tasks/config/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,12 @@ module.exports = function(grunt) {
args: ['scripts/notice', '--validate'],
}),

test_hardening: scriptWithGithubChecks({
title: 'Node.js hardening tests',
cmd: NODE,
args: ['scripts/test_hardening.js'],
}),

apiIntegrationTests: scriptWithGithubChecks({
title: 'API integration tests',
cmd: NODE,
Expand Down
1 change: 1 addition & 0 deletions tasks/jenkins.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ module.exports = function(grunt) {
'run:test_jest_integration',
'run:test_projects',
'run:test_karma_ci',
'run:test_hardening',
'run:apiIntegrationTests',
]);
};
3 changes: 3 additions & 0 deletions test/harden/_echo.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/usr/bin/env sh

echo $POLLUTED$custom
20 changes: 20 additions & 0 deletions test/harden/_fork.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

console.log(`${process.env.POLLUTED || ''}${process.env.custom || ''}`);
Loading