Skip to content

Commit

Permalink
Patch lodash template (#12)
Browse files Browse the repository at this point in the history
* Applying changes from elastic#64985

* Using isIterateeCall, because it seems less brittle

* Also patching `lodash/template` and `lodash/fp/template`

* Reorganizing some files...

* Revising comment
  • Loading branch information
kobelb authored Jul 2, 2020
1 parent 15bbfee commit dc10eb0
Show file tree
Hide file tree
Showing 4 changed files with 258 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
var hook = require('require-in-the-middle');

// 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) {
hook(['child_process'], function (cp) {
// 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 uncomment the line below.
Expand All @@ -36,7 +37,7 @@ module.exports = function (cp) {
cp.spawnSync = new Proxy(cp.spawnSync, { apply: patchOptions(true) });

return cp;
};
});

function patchOptions(hasArgs) {
return function apply(target, thisArg, args) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,5 @@
* 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
});
require('./child_process');
require('./lodash_template');
72 changes: 72 additions & 0 deletions src/setup_node_env/harden/lodash_template.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* 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');
var isIterateeCall = require('lodash/_isIterateeCall');

hook(['lodash'], function (lodash) {
return {
...lodash,
template: createProxy(lodash.template),
};
});

hook(['lodash/template'], function (template) {
return createProxy(template);
});

hook(['lodash/fp'], function (fp) {
return {
...fp,
template: createFpProxy(fp.template),
};
});

hook(['lodash/fp/template'], function (template) {
return createFpProxy(template);
});

function createProxy(template) {
return new Proxy(template, {
apply: function (target, thisArg, args) {
if (args.length === 1 || isIterateeCall(args)) {
return target.apply(thisArg, [args[0], { sourceURL: '' }]);
}

var options = Object.assign({}, args[1]);
options.sourceURL = (options.sourceURL + '').replace(/\s/g, ' ');
var newArgs = args.slice(0); // copy
newArgs.splice(1, 1, options); // replace options in the copy
return target.apply(thisArg, newArgs);
},
});
}

function createFpProxy(template) {
// we have to do the require here, so that we get the patched version
var _ = require('lodash');
return new Proxy(template, {
apply: function (target, thisArg, args) {
// per https://github.com/lodash/lodash/wiki/FP-Guide
// > Iteratee arguments are capped to avoid gotchas with variadic iteratees.
// this means that we can't specify the options in the second argument to fp.template because it's ignored.
// Instead, we're going to use the non-FP _.template with only the first argument which has already been patched
return _.template(args[0]);
},
});
}
181 changes: 181 additions & 0 deletions test/harden/lodash_template.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
/*
* 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.
*/

require('../../src/setup_node_env');
const _ = require('lodash');
const template = require('lodash/template');
const fp = require('lodash/fp');
const fpTemplate = require('lodash/fp/template');
const test = require('tape');

Object.prototype.sourceURL = '\u2028\u2029\n;global.whoops=true'; // eslint-disable-line no-extend-native

test.onFinish(() => {
delete Object.prototype.sourceURL;
});

test('test setup ok', (t) => {
t.equal({}.sourceURL, '\u2028\u2029\n;global.whoops=true');
t.end();
});

[_.template, template].forEach((fn) => {
test(`_.template('<%= foo %>')`, (t) => {
const output = fn('<%= foo %>')({ foo: 'bar' });
t.equal(output, 'bar');
t.equal(global.whoops, undefined);
t.end();
});

test(`_.template('<%= foo %>', {})`, (t) => {
const output = fn('<%= foo %>', Object.freeze({}))({ foo: 'bar' });
t.equal(output, 'bar');
t.equal(global.whoops, undefined);
t.end();
});

test(`_.template('<%= data.foo %>', { variable: 'data' })`, (t) => {
const output = fn('<%= data.foo %>', Object.freeze({ variable: 'data' }))({ foo: 'bar' });
t.equal(output, 'bar');
t.equal(global.whoops, undefined);
t.end();
});

test(`_.template('<%= foo %>', { sourceURL: '/foo/bar' })`, (t) => {
// throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do
const template = fn('<% throw new Error() %>', Object.freeze({ sourceURL: '/foo/bar' }));
t.plan(2);
try {
template();
} catch (err) {
const path = parsePathFromStack(err.stack);
t.equal(path, '/foo/bar');
t.equal(global.whoops, undefined);
}
});

test(`_.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true' })`, (t) => {
// throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do
const template = fn(
'<% throw new Error() %>',
Object.freeze({ sourceURL: '\u2028\u2029\nglobal.whoops=true' })
);
t.plan(2);
try {
template();
} catch (err) {
const path = parsePathFromStack(err.stack);
t.equal(path, 'global.whoops=true');
t.equal(global.whoops, undefined);
}
});

test(`_.template used as an iteratee call(`, (t) => {
const templateStrArr = ['<%= data.foo %>', 'example <%= data.foo %>'];
const output = _.map(templateStrArr, fn);

t.equal(output[0]({ data: { foo: 'bar' } }), 'bar');
t.equal(output[1]({ data: { foo: 'bar' } }), 'example bar');
t.equal(global.whoops, undefined);
t.end();
});
});

[fp.template, fpTemplate].forEach((fn) => {
test(`fp.template('<%= foo %>')`, (t) => {
const output = fn('<%= foo %>')({ foo: 'bar' });
t.equal(output, 'bar');
t.equal(global.whoops, undefined);
t.end();
});

test(`fp.template('<%= foo %>', {})`, (t) => {
// fp.template ignores the second argument, this is negligible in this situation since options is an empty object
const output = fn('<%= foo %>', Object.freeze({}))({ foo: 'bar' });
t.equal(output, 'bar');
t.equal(global.whoops, undefined);
t.end();
});

test(`fp.template('<%= data.foo %>', { variable: 'data' })`, (t) => {
// fp.template ignores the second argument, this causes an error to be thrown
t.plan(2);
try {
fn('<%= data.foo %>', Object.freeze({ variable: 'data' }))({ foo: 'bar' });
} catch (err) {
t.equal(err.message, 'data is not defined');
t.equal(global.whoops, undefined);
}
});

test(`fp.template('<%= foo %>', { sourceURL: '/foo/bar' })`, (t) => {
// fp.template ignores the second argument, the sourceURL is ignored
// throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do
// our patching to hard-code the sourceURL and use non-FP _.template does slightly alter the stack-traces but it's negligible
const template = fn('<% throw new Error() %>', Object.freeze({ sourceURL: '/foo/bar' }));
t.plan(3);
try {
template();
} catch (err) {
const path = parsePathFromStack(err.stack);
t.match(path, /^eval at <anonymous> /);
t.doesNotMatch(path, /\/foo\/bar/);
t.equal(global.whoops, undefined);
}
});

test(`fp.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true' })`, (t) => {
// fp.template ignores the second argument, the sourceURL is ignored
// throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do
// our patching to hard-code the sourceURL and use non-FP _.template does slightly alter the stack-traces but it's negligible
const template = fn(
'<% throw new Error() %>',
Object.freeze({ sourceURL: '\u2028\u2029\nglobal.whoops=true' })
);
t.plan(3);
try {
template();
} catch (err) {
const path = parsePathFromStack(err.stack);
t.match(path, /^eval at <anonymous> /);
t.doesNotMatch(path, /\/foo\/bar/);
t.equal(global.whoops, undefined);
}
});

test(`fp.template used as an iteratee call(`, (t) => {
const templateStrArr = ['<%= data.foo %>', 'example <%= data.foo %>'];
const output = fp.map(fn)(templateStrArr);

t.equal(output[0]({ data: { foo: 'bar' } }), 'bar');
t.equal(output[1]({ data: { foo: 'bar' } }), 'example bar');
t.equal(global.whoops, undefined);
t.end();
});
});

function parsePathFromStack(stack) {
const lines = stack.split('\n');
// the frame starts at the second line
const frame = lines[1];

// the path is in parathensis, and ends with a colon before the line/column numbers
const [, path] = /\(([^:]+)/.exec(frame);
return path;
}

0 comments on commit dc10eb0

Please sign in to comment.