From 01dfef735fd236d4db2abad64042cf6e9bc1fb7e Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 14 Oct 2021 11:51:05 +0100 Subject: [PATCH] Use vm2 for JS execution in node, and a vm polyfill for the browser. Use 2 standalone entrypoints for string-templates depending on env --- packages/string-templates/package.json | 3 +- packages/string-templates/rollup.config.js | 36 +--- .../src/helpers/javascript.js | 54 +---- packages/string-templates/src/index.cjs | 185 ++-------------- packages/string-templates/src/index.js | 204 ++++++++++++++++++ packages/string-templates/src/index.mjs | 14 +- packages/string-templates/src/utilities.js | 8 + .../string-templates/test/javascript.spec.js | 26 ++- packages/string-templates/yarn.lock | 5 + 9 files changed, 283 insertions(+), 252 deletions(-) create mode 100644 packages/string-templates/src/index.js diff --git a/packages/string-templates/package.json b/packages/string-templates/package.json index dcb734eaa8..4ae67e0516 100644 --- a/packages/string-templates/package.json +++ b/packages/string-templates/package.json @@ -24,7 +24,8 @@ "dayjs": "^1.10.4", "handlebars": "^4.7.6", "handlebars-utils": "^1.0.6", - "lodash": "^4.17.20" + "lodash": "^4.17.20", + "vm2": "^3.9.4" }, "devDependencies": { "@rollup/plugin-commonjs": "^17.1.0", diff --git a/packages/string-templates/rollup.config.js b/packages/string-templates/rollup.config.js index 4685063b31..5625f8180c 100644 --- a/packages/string-templates/rollup.config.js +++ b/packages/string-templates/rollup.config.js @@ -7,18 +7,6 @@ import globals from "rollup-plugin-node-globals" const production = !process.env.ROLLUP_WATCH -const plugins = [ - resolve({ - preferBuiltins: true, - browser: true, - }), - commonjs(), - globals(), - builtins(), - json(), - production && terser(), -] - export default [ { input: "src/index.mjs", @@ -27,18 +15,16 @@ export default [ format: "esm", file: "./dist/bundle.mjs", }, - plugins, + plugins: [ + resolve({ + preferBuiltins: true, + browser: true, + }), + commonjs(), + globals(), + builtins(), + json(), + production && terser(), + ], }, - // This is the valid configuration for a CommonJS bundle, but since we have - // no use for this, it's better to leave it out. - // { - // input: "src/index.cjs", - // output: { - // sourcemap: !production, - // format: "cjs", - // file: "./dist/bundle.cjs", - // exports: "named", - // }, - // plugins, - // }, ] diff --git a/packages/string-templates/src/helpers/javascript.js b/packages/string-templates/src/helpers/javascript.js index 2c2802c64c..e96212ecb6 100644 --- a/packages/string-templates/src/helpers/javascript.js +++ b/packages/string-templates/src/helpers/javascript.js @@ -1,5 +1,9 @@ -const CAPTURE_JS = new RegExp(/{{ js "(.*)" }}/) -const vm = require("vm") +const { atob } = require("../utilities") + +// The method of executing JS scripts depends on the bundle being built. +// This setter is used in the entrypoint (either index.cjs or index.mjs). +let runJS +module.exports.setJSRunner = runner => (runJS = runner) // Helper utility to strip square brackets from a value const removeSquareBrackets = value => { @@ -27,23 +31,8 @@ const getContextValue = (path, context) => { return data } -// Node polyfill for base64 encoding -const btoa = plainText => { - return Buffer.from(plainText, "utf-8").toString("base64") -} - -// Node polyfill for base64 decoding -const atob = base64 => { - return Buffer.from(base64, "base64").toString("utf-8") -} - // Evaluates JS code against a certain context module.exports.processJS = (handlebars, context) => { - // Do not evaluate JS in a node environment - if (typeof window === "undefined") { - return "JS bindings are not executed in a Node environment" - } - try { // Wrap JS in a function and immediately invoke it. // This is required to allow the final `return` statement to be valid. @@ -58,37 +47,8 @@ module.exports.processJS = (handlebars, context) => { } // Create a sandbox with out context and run the JS - vm.createContext(sandboxContext) - return vm.runInNewContext(js, sandboxContext, { timeout: 1000 }) + return runJS(js, sandboxContext) } catch (error) { return "Error while executing JS" } } - -// Checks if a HBS expression is a valid JS HBS expression -module.exports.isJSBinding = handlebars => { - return module.exports.decodeJSBinding(handlebars) != null -} - -// Encodes a raw JS string as a JS HBS expression -module.exports.encodeJSBinding = javascript => { - return `{{ js "${btoa(javascript)}" }}` -} - -// Decodes a JS HBS expression to the raw JS string -module.exports.decodeJSBinding = handlebars => { - if (!handlebars || typeof handlebars !== "string") { - return null - } - - // JS is only valid if it is the only HBS expression - if (!handlebars.trim().startsWith("{{ js ")) { - return null - } - - const match = handlebars.match(CAPTURE_JS) - if (!match || match.length < 2) { - return null - } - return atob(match[1]) -} diff --git a/packages/string-templates/src/index.cjs b/packages/string-templates/src/index.cjs index 31de252381..5d05f0f57f 100644 --- a/packages/string-templates/src/index.cjs +++ b/packages/string-templates/src/index.cjs @@ -1,169 +1,28 @@ -const handlebars = require("handlebars") -const { registerAll } = require("./helpers/index") -const processors = require("./processors") -const { removeHandlebarsStatements } = require("./utilities") -const manifest = require("../manifest.json") -const JS = require("./helpers/javascript") - -const hbsInstance = handlebars.create() -registerAll(hbsInstance) +const { VM } = require("vm2") +const templates = require("./index.js") +const { setJSRunner } = require("./helpers/javascript") /** - * utility function to check if the object is valid + * CJS entrypoint for rollup */ -function testObject(object) { - // JSON stringify will fail if there are any cycles, stops infinite recursion - try { - JSON.stringify(object) - } catch (err) { - throw "Unable to process inputs to JSON, cannot recurse" - } -} +module.exports.isValid = templates.isValid +module.exports.makePropSafe = templates.makePropSafe +module.exports.getManifest = templates.getManifest +module.exports.isJSBinding = templates.isJSBinding +module.exports.encodeJSBinding = templates.encodeJSBinding +module.exports.decodeJSBinding = templates.decodeJSBinding +module.exports.processStringSync = templates.processStringSync +module.exports.processObjectSync = templates.processObjectSync +module.exports.processString = templates.processString +module.exports.processObject = templates.processObject /** - * Given an input object this will recurse through all props to try and update any handlebars statements within. - * @param {object|array} object The input structure which is to be recursed, it is important to note that - * if the structure contains any cycles then this will fail. - * @param {object} context The context that handlebars should fill data from. - * @returns {Promise} The structure input, as fully updated as possible. + * Use vm2 to run JS scripts in a node env */ -module.exports.processObject = async (object, context) => { - testObject(object) - for (let key of Object.keys(object || {})) { - if (object[key] != null) { - let val = object[key] - if (typeof val === "string") { - object[key] = await module.exports.processString(object[key], context) - } else if (typeof val === "object") { - object[key] = await module.exports.processObject(object[key], context) - } - } - } - return object -} - -/** - * This will process a single handlebars containing string. If the string passed in has no valid handlebars statements - * then nothing will occur. - * @param {string} string The template string which is the filled from the context object. - * @param {object} context An object of information which will be used to enrich the string. - * @returns {Promise} The enriched string, all templates should have been replaced if they can be. - */ -module.exports.processString = async (string, context) => { - // TODO: carry out any async calls before carrying out async call - return module.exports.processStringSync(string, context) -} - -/** - * Given an input object this will recurse through all props to try and update any handlebars statements within. This is - * a pure sync call and therefore does not have the full functionality of the async call. - * @param {object|array} object The input structure which is to be recursed, it is important to note that - * if the structure contains any cycles then this will fail. - * @param {object} context The context that handlebars should fill data from. - * @returns {object|array} The structure input, as fully updated as possible. - */ -module.exports.processObjectSync = (object, context) => { - testObject(object) - for (let key of Object.keys(object || {})) { - let val = object[key] - if (typeof val === "string") { - object[key] = module.exports.processStringSync(object[key], context) - } else if (typeof val === "object") { - object[key] = module.exports.processObjectSync(object[key], context) - } - } - return object -} - -/** - * This will process a single handlebars containing string. If the string passed in has no valid handlebars statements - * then nothing will occur. This is a pure sync call and therefore does not have the full functionality of the async call. - * @param {string} string The template string which is the filled from the context object. - * @param {object} context An object of information which will be used to enrich the string. - * @returns {string} The enriched string, all templates should have been replaced if they can be. - */ -module.exports.processStringSync = (string, context) => { - if (!exports.isValid(string)) { - return string - } - // take a copy of input incase error - const input = string - if (typeof string !== "string") { - throw "Cannot process non-string types." - } - try { - string = processors.preprocess(string) - // this does not throw an error when template can't be fulfilled, have to try correct beforehand - const template = hbsInstance.compile(string, { - strict: false, - }) - return processors.postprocess(template({ - now: new Date().toISOString(), - ...context, - })) - } catch (err) { - return removeHandlebarsStatements(input) - } -} - -/** - * Simple utility function which makes sure that a templating property has been wrapped in literal specifiers correctly. - * @param {string} property The property which is to be wrapped. - * @returns {string} The wrapped property ready to be added to a templating string. - */ -module.exports.makePropSafe = property => { - return `[${property}]`.replace("[[", "[").replace("]]", "]") -} - -/** - * Checks whether or not a template string contains totally valid syntax (simply tries running it) - * @param string The string to test for valid syntax - this may contain no templates and will be considered valid. - * @returns {boolean} Whether or not the input string is valid. - */ -module.exports.isValid = string => { - const validCases = [ - "string", - "number", - "object", - "array", - "cannot read property", - "undefined", - ] - // this is a portion of a specific string always output by handlebars in the case of a syntax error - const invalidCases = [`expecting '`] - // don't really need a real context to check if its valid - const context = {} - try { - hbsInstance.compile(processors.preprocess(string, false))(context) - return true - } catch (err) { - const msg = err && err.message ? err.message : err - if (!msg) { - return false - } - const invalidCase = invalidCases.some(invalidCase => - msg.toLowerCase().includes(invalidCase) - ) - const validCase = validCases.some(validCase => - msg.toLowerCase().includes(validCase) - ) - // special case for maths functions - don't have inputs yet - return validCase && !invalidCase - } -} - -/** - * We have generated a static manifest file from the helpers that this string templating package makes use of. - * This manifest provides information about each of the helpers and how it can be used. - * @returns The manifest JSON which has been generated from the helpers. - */ -module.exports.getManifest = () => { - return manifest -} - -/** - * Export utilities for working with JS bindings - */ -module.exports.isJSBinding = JS.isJSBinding -module.exports.decodeJSBinding = JS.decodeJSBinding -module.exports.encodeJSBinding = JS.encodeJSBinding \ No newline at end of file +setJSRunner((js, context) => { + const vm = new VM({ + sandbox: context, + timeout: 1000 + }) + return vm.run(js) +}) \ No newline at end of file diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js new file mode 100644 index 0000000000..590469fd5d --- /dev/null +++ b/packages/string-templates/src/index.js @@ -0,0 +1,204 @@ +const handlebars = require("handlebars") +const { registerAll } = require("./helpers/index") +const processors = require("./processors") +const { removeHandlebarsStatements, atob, btoa } = require("./utilities") +const manifest = require("../manifest.json") + +const hbsInstance = handlebars.create() +registerAll(hbsInstance) + +/** + * utility function to check if the object is valid + */ +function testObject(object) { + // JSON stringify will fail if there are any cycles, stops infinite recursion + try { + JSON.stringify(object) + } catch (err) { + throw "Unable to process inputs to JSON, cannot recurse" + } +} + +/** + * Given an input object this will recurse through all props to try and update any handlebars statements within. + * @param {object|array} object The input structure which is to be recursed, it is important to note that + * if the structure contains any cycles then this will fail. + * @param {object} context The context that handlebars should fill data from. + * @returns {Promise} The structure input, as fully updated as possible. + */ +module.exports.processObject = async (object, context) => { + testObject(object) + for (let key of Object.keys(object || {})) { + if (object[key] != null) { + let val = object[key] + if (typeof val === "string") { + object[key] = await module.exports.processString(object[key], context) + } else if (typeof val === "object") { + object[key] = await module.exports.processObject(object[key], context) + } + } + } + return object +} + +/** + * This will process a single handlebars containing string. If the string passed in has no valid handlebars statements + * then nothing will occur. + * @param {string} string The template string which is the filled from the context object. + * @param {object} context An object of information which will be used to enrich the string. + * @returns {Promise} The enriched string, all templates should have been replaced if they can be. + */ +module.exports.processString = async (string, context) => { + // TODO: carry out any async calls before carrying out async call + return module.exports.processStringSync(string, context) +} + +/** + * Given an input object this will recurse through all props to try and update any handlebars statements within. This is + * a pure sync call and therefore does not have the full functionality of the async call. + * @param {object|array} object The input structure which is to be recursed, it is important to note that + * if the structure contains any cycles then this will fail. + * @param {object} context The context that handlebars should fill data from. + * @returns {object|array} The structure input, as fully updated as possible. + */ +module.exports.processObjectSync = (object, context) => { + testObject(object) + for (let key of Object.keys(object || {})) { + let val = object[key] + if (typeof val === "string") { + object[key] = module.exports.processStringSync(object[key], context) + } else if (typeof val === "object") { + object[key] = module.exports.processObjectSync(object[key], context) + } + } + return object +} + +/** + * This will process a single handlebars containing string. If the string passed in has no valid handlebars statements + * then nothing will occur. This is a pure sync call and therefore does not have the full functionality of the async call. + * @param {string} string The template string which is the filled from the context object. + * @param {object} context An object of information which will be used to enrich the string. + * @returns {string} The enriched string, all templates should have been replaced if they can be. + */ +module.exports.processStringSync = (string, context) => { + if (!exports.isValid(string)) { + return string + } + // take a copy of input incase error + const input = string + if (typeof string !== "string") { + throw "Cannot process non-string types." + } + try { + string = processors.preprocess(string) + // this does not throw an error when template can't be fulfilled, have to try correct beforehand + const template = hbsInstance.compile(string, { + strict: false, + }) + return processors.postprocess( + template({ + now: new Date().toISOString(), + ...context, + }) + ) + } catch (err) { + return removeHandlebarsStatements(input) + } +} + +/** + * Simple utility function which makes sure that a templating property has been wrapped in literal specifiers correctly. + * @param {string} property The property which is to be wrapped. + * @returns {string} The wrapped property ready to be added to a templating string. + */ +module.exports.makePropSafe = property => { + return `[${property}]`.replace("[[", "[").replace("]]", "]") +} + +/** + * Checks whether or not a template string contains totally valid syntax (simply tries running it) + * @param string The string to test for valid syntax - this may contain no templates and will be considered valid. + * @returns {boolean} Whether or not the input string is valid. + */ +module.exports.isValid = string => { + const validCases = [ + "string", + "number", + "object", + "array", + "cannot read property", + "undefined", + ] + // this is a portion of a specific string always output by handlebars in the case of a syntax error + const invalidCases = [`expecting '`] + // don't really need a real context to check if its valid + const context = {} + try { + hbsInstance.compile(processors.preprocess(string, false))(context) + return true + } catch (err) { + const msg = err && err.message ? err.message : err + if (!msg) { + return false + } + const invalidCase = invalidCases.some(invalidCase => + msg.toLowerCase().includes(invalidCase) + ) + const validCase = validCases.some(validCase => + msg.toLowerCase().includes(validCase) + ) + // special case for maths functions - don't have inputs yet + return validCase && !invalidCase + } +} + +/** + * We have generated a static manifest file from the helpers that this string templating package makes use of. + * This manifest provides information about each of the helpers and how it can be used. + * @returns The manifest JSON which has been generated from the helpers. + */ +module.exports.getManifest = () => { + return manifest +} + +/** + * Checks if a HBS expression is a valid JS HBS expression + * @param handlebars the HBS expression to check + * @returns {boolean} whether the expression is JS or not + */ +module.exports.isJSBinding = handlebars => { + return module.exports.decodeJSBinding(handlebars) != null +} + +/** + * Encodes a raw JS string as a JS HBS expression + * @param javascript the JS code to encode + * @returns {string} the JS HBS expression + */ +module.exports.encodeJSBinding = javascript => { + return `{{ js "${btoa(javascript)}" }}` +} + +/** + * Decodes a JS HBS expression to the raw JS code + * @param handlebars the JS HBS expression + * @returns {string|null} the raw JS code + */ +module.exports.decodeJSBinding = handlebars => { + if (!handlebars || typeof handlebars !== "string") { + return null + } + + // JS is only valid if it is the only HBS expression + if (!handlebars.trim().startsWith("{{ js ")) { + return null + } + + const captureJSRegex = new RegExp(/{{ js "(.*)" }}/) + const match = handlebars.match(captureJSRegex) + if (!match || match.length < 2) { + return null + } + return atob(match[1]) +} diff --git a/packages/string-templates/src/index.mjs b/packages/string-templates/src/index.mjs index 855bd97a55..abb74d4ac4 100644 --- a/packages/string-templates/src/index.mjs +++ b/packages/string-templates/src/index.mjs @@ -1,7 +1,9 @@ -import templates from "./index.cjs" +import vm from "vm" +import templates from "./index.js" +import { setJSRunner } from "./helpers/javascript" /** - * This file is simply an entrypoint for rollup - makes a lot of cjs problems go away + * ES6 entrypoint for rollup */ export const isValid = templates.isValid export const makePropSafe = templates.makePropSafe @@ -13,3 +15,11 @@ export const processStringSync = templates.processStringSync export const processObjectSync = templates.processObjectSync export const processString = templates.processString export const processObject = templates.processObject + +/** + * Use polyfilled vm to run JS scripts in a browser Env + */ +setJSRunner((js, context) => { + vm.createContext(context) + return vm.runInNewContext(js, context, { timeout: 1000 }) +}) \ No newline at end of file diff --git a/packages/string-templates/src/utilities.js b/packages/string-templates/src/utilities.js index 3dc83b1fdb..50d770c6ea 100644 --- a/packages/string-templates/src/utilities.js +++ b/packages/string-templates/src/utilities.js @@ -22,3 +22,11 @@ module.exports.removeHandlebarsStatements = string => { } return string } + +module.exports.btoa = plainText => { + return Buffer.from(plainText, "utf-8").toString("base64") +} + +module.exports.atob = base64 => { + return Buffer.from(base64, "base64").toString("utf-8") +} diff --git a/packages/string-templates/test/javascript.spec.js b/packages/string-templates/test/javascript.spec.js index 07cc72c298..05cc80331a 100644 --- a/packages/string-templates/test/javascript.spec.js +++ b/packages/string-templates/test/javascript.spec.js @@ -4,21 +4,7 @@ const processJS = (js, context) => { return processStringSync(encodeJSBinding(js), context) } -describe("Test the JavaScript helper in Node", () => { - it("should not execute JS in Node", () => { - const output = processJS(`return 1`) - expect(output).toBe("JS bindings are not executed in a Node environment") - }) -}) - describe("Test the JavaScript helper", () => { - // JS bindings do not get evaluated on the server for safety. - // Since we want to run SJ for tests, we fake a window object to make - // it think that we're in the browser - beforeEach(() => { - window = {} - }) - it("should execute a simple expression", () => { const output = processJS(`return 1 + 2`) expect(output).toBe("3") @@ -84,4 +70,16 @@ describe("Test the JavaScript helper", () => { const output = processJS(`while (true) {}`) expect(output).toBe("Error while executing JS") }) + + it("should prevent access to the process global", () => { + const output = processJS(`return process`) + expect(output).toBe("Error while executing JS") + }) + + it("should prevent sandbox escape", () => { + const output = processJS( + `return this.constructor.constructor("return process")()` + ) + expect(output).toBe("Error while executing JS") + }) }) diff --git a/packages/string-templates/yarn.lock b/packages/string-templates/yarn.lock index b5abb1403a..96ee466b3a 100644 --- a/packages/string-templates/yarn.lock +++ b/packages/string-templates/yarn.lock @@ -4572,6 +4572,11 @@ vlq@^0.2.2: resolved "https://registry.yarnpkg.com/vlq/-/vlq-0.2.3.tgz#8f3e4328cf63b1540c0d67e1b2778386f8975b26" integrity sha512-DRibZL6DsNhIgYQ+wNdWDL2SL3bKPlVrRiBqV5yuMm++op8W4kGFtaQfCs4KEJn0wBZcHVHJ3eoywX8983k1ow== +vm2@^3.9.4: + version "3.9.4" + resolved "https://registry.yarnpkg.com/vm2/-/vm2-3.9.4.tgz#2e118290fefe7bd8ea09ebe2f5faf53730dbddaa" + integrity sha512-sOdharrJ7KEePIpHekiWaY1DwgueuiBeX/ZBJUPgETsVlJsXuEx0K0/naATq2haFvJrvZnRiORQRubR0b7Ye6g== + w3c-hr-time@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/w3c-hr-time/-/w3c-hr-time-1.0.2.tgz#0a89cdf5cc15822df9c360543676963e0cc308cd"