diff --git a/packages/backend-core/src/context/mainContext.ts b/packages/backend-core/src/context/mainContext.ts index 983a4d20e1..e439eb5fd0 100644 --- a/packages/backend-core/src/context/mainContext.ts +++ b/packages/backend-core/src/context/mainContext.ts @@ -335,3 +335,11 @@ export function isScim(): boolean { const scimCall = context?.isScim return !!scimCall } + +export function getCurrentContext(): ContextMap | undefined { + try { + return Context.get() + } catch (e) { + return undefined + } +} diff --git a/packages/backend-core/src/context/types.ts b/packages/backend-core/src/context/types.ts index a1606a17b9..f73dc9f5c7 100644 --- a/packages/backend-core/src/context/types.ts +++ b/packages/backend-core/src/context/types.ts @@ -1,4 +1,5 @@ import { IdentityContext } from "@budibase/types" +import { ExecutionTimeTracker } from "../timers" // keep this out of Budibase types, don't want to expose context info export type ContextMap = { @@ -9,4 +10,5 @@ export type ContextMap = { isScim?: boolean automationId?: string isMigrating?: boolean + jsExecutionTracker?: ExecutionTimeTracker } diff --git a/packages/backend-core/src/timers/timers.ts b/packages/backend-core/src/timers/timers.ts index 000be74821..3bb556703f 100644 --- a/packages/backend-core/src/timers/timers.ts +++ b/packages/backend-core/src/timers/timers.ts @@ -20,3 +20,37 @@ export function cleanup() { } intervals = [] } + +export class ExecutionTimeoutError extends Error { + public readonly name = "ExecutionTimeoutError" +} + +export class ExecutionTimeTracker { + static withLimit(limitMs: number) { + return new ExecutionTimeTracker(limitMs) + } + + constructor(private limitMs: number) {} + + private totalTimeMs = 0 + + track(f: () => T): T { + this.checkLimit() + const start = process.hrtime.bigint() + try { + return f() + } finally { + const end = process.hrtime.bigint() + this.totalTimeMs += Number(end - start) / 1e6 + this.checkLimit() + } + } + + private checkLimit() { + if (this.totalTimeMs > this.limitMs) { + throw new ExecutionTimeoutError( + `Execution time limit of ${this.limitMs}ms exceeded: ${this.totalTimeMs}ms` + ) + } + } +} diff --git a/packages/backend-core/src/utils/Duration.ts b/packages/backend-core/src/utils/Duration.ts index 3c7ef23b11..730b59d1dc 100644 --- a/packages/backend-core/src/utils/Duration.ts +++ b/packages/backend-core/src/utils/Duration.ts @@ -49,4 +49,8 @@ export class Duration { static fromDays(duration: number) { return Duration.from(DurationType.DAYS, duration) } + + static fromMilliseconds(duration: number) { + return Duration.from(DurationType.MILLISECONDS, duration) + } } diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 5b3c69b87a..2d47e5f54a 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -2086,4 +2086,112 @@ describe.each([ expect(row.formula).toBe(relatedRow.name) }) }) + + describe("Formula JS protection", () => { + it("should time out JS execution if a single cell takes too long", async () => { + await config.withEnv({ JS_PER_EXECUTION_TIME_LIMIT_MS: 20 }, async () => { + const js = Buffer.from( + ` + let i = 0; + while (true) { + i++; + } + return i; + ` + ).toString("base64") + + const table = await config.createTable({ + name: "table", + type: "table", + schema: { + text: { + name: "text", + type: FieldType.STRING, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: `{{ js "${js}"}}`, + formulaType: FormulaTypes.DYNAMIC, + }, + }, + }) + + await config.api.row.save(table._id!, { text: "foo" }) + const { rows } = await config.api.row.search(table._id!) + expect(rows).toHaveLength(1) + const row = rows[0] + expect(row.text).toBe("foo") + expect(row.formula).toBe("Timed out while executing JS") + }) + }) + + it("should time out JS execution if a multiple cells take too long", async () => { + await config.withEnv( + { + JS_PER_EXECUTION_TIME_LIMIT_MS: 20, + JS_PER_REQUEST_TIME_LIMIT_MS: 40, + }, + async () => { + const js = Buffer.from( + ` + let i = 0; + while (true) { + i++; + } + return i; + ` + ).toString("base64") + + const table = await config.createTable({ + name: "table", + type: "table", + schema: { + text: { + name: "text", + type: FieldType.STRING, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: `{{ js "${js}"}}`, + formulaType: FormulaTypes.DYNAMIC, + }, + }, + }) + + for (let i = 0; i < 10; i++) { + await config.api.row.save(table._id!, { text: "foo" }) + } + + // Run this test 3 times to make sure that there's no cross-request + // pollution of the execution time tracking. + for (let reqs = 0; reqs < 3; reqs++) { + const { rows } = await config.api.row.search(table._id!) + expect(rows).toHaveLength(10) + + let i = 0 + for (; i < 10; i++) { + const row = rows[i] + if (row.formula !== "Timed out while executing JS") { + break + } + } + + // Given the execution times are not deterministic, we can't be sure + // of the exact number of rows that were executed before the timeout + // but it should absolutely be at least 1. + expect(i).toBeGreaterThan(0) + expect(i).toBeLessThan(5) + + for (; i < 10; i++) { + const row = rows[i] + expect(row.text).toBe("foo") + expect(row.formula).toBe("Request JS execution limit hit") + } + } + } + ) + }) + }) }) diff --git a/packages/server/src/environment.ts b/packages/server/src/environment.ts index 98a90fbddc..f692a8b6cf 100644 --- a/packages/server/src/environment.ts +++ b/packages/server/src/environment.ts @@ -70,6 +70,11 @@ const environment = { SELF_HOSTED: process.env.SELF_HOSTED, HTTP_MB_LIMIT: process.env.HTTP_MB_LIMIT, FORKED_PROCESS_NAME: process.env.FORKED_PROCESS_NAME || "main", + JS_PER_EXECUTION_TIME_LIMIT_MS: + parseIntSafe(process.env.JS_PER_EXECUTION_TIME_LIMIT_MS) || 1000, + JS_PER_REQUEST_TIME_LIMIT_MS: parseIntSafe( + process.env.JS_PER_REQUEST_TIME_LIMIT_MS + ), // old CLIENT_ID: process.env.CLIENT_ID, _set(key: string, value: any) { diff --git a/packages/server/src/jsRunner.ts b/packages/server/src/jsRunner.ts new file mode 100644 index 0000000000..a9301feb60 --- /dev/null +++ b/packages/server/src/jsRunner.ts @@ -0,0 +1,36 @@ +import vm from "vm" +import env from "./environment" +import { setJSRunner } from "@budibase/string-templates" +import { context, timers } from "@budibase/backend-core" + +type TrackerFn = (f: () => T) => T + +export function init() { + setJSRunner((js: string, ctx: vm.Context) => { + const perRequestLimit = env.JS_PER_REQUEST_TIME_LIMIT_MS + let track: TrackerFn = f => f() + if (perRequestLimit) { + const bbCtx = context.getCurrentContext() + if (bbCtx) { + if (!bbCtx.jsExecutionTracker) { + bbCtx.jsExecutionTracker = + timers.ExecutionTimeTracker.withLimit(perRequestLimit) + } + track = bbCtx.jsExecutionTracker.track.bind(bbCtx.jsExecutionTracker) + } + } + + ctx = { + ...ctx, + alert: undefined, + setInterval: undefined, + setTimeout: undefined, + } + vm.createContext(ctx) + return track(() => + vm.runInNewContext(js, ctx, { + timeout: env.JS_PER_EXECUTION_TIME_LIMIT_MS, + }) + ) + }) +} diff --git a/packages/server/src/startup.ts b/packages/server/src/startup.ts index 2db6e5ae6a..6860fe5f9b 100644 --- a/packages/server/src/startup.ts +++ b/packages/server/src/startup.ts @@ -23,6 +23,7 @@ import { automationsEnabled, printFeatures } from "./features" import Koa from "koa" import { Server } from "http" import { AddressInfo } from "net" +import * as jsRunner from "./jsRunner" let STARTUP_RAN = false @@ -152,4 +153,6 @@ export async function startup(app?: Koa, server?: Server) { } }) } + + jsRunner.init() } diff --git a/packages/string-templates/src/helpers/javascript.js b/packages/string-templates/src/helpers/javascript.js index 53baec8613..585a2ba6b7 100644 --- a/packages/string-templates/src/helpers/javascript.js +++ b/packages/string-templates/src/helpers/javascript.js @@ -56,10 +56,12 @@ module.exports.processJS = (handlebars, context) => { const res = { data: runJS(js, sandboxContext) } return `{{${LITERAL_MARKER} js_result-${JSON.stringify(res)}}}` } catch (error) { - console.log(`JS error: ${typeof error} ${JSON.stringify(error)}`) if (error.code === "ERR_SCRIPT_EXECUTION_TIMEOUT") { return "Timed out while executing JS" } + if (error.name === "ExecutionTimeoutError") { + return "Request JS execution limit hit" + } return "Error while executing JS" } } diff --git a/packages/string-templates/src/index.cjs b/packages/string-templates/src/index.cjs index 1cf3c8d0c6..aedb7fc052 100644 --- a/packages/string-templates/src/index.cjs +++ b/packages/string-templates/src/index.cjs @@ -18,6 +18,7 @@ module.exports.doesContainString = templates.doesContainString module.exports.disableEscaping = templates.disableEscaping module.exports.findHBSBlocks = templates.findHBSBlocks module.exports.convertToJS = templates.convertToJS +module.exports.setJSRunner = templates.setJSRunner module.exports.FIND_ANY_HBS_REGEX = templates.FIND_ANY_HBS_REGEX if (!process.env.NO_JS) { diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index 4844bf5fb6..63da7fde4d 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -9,6 +9,7 @@ const { findDoubleHbsInstances, } = require("./utilities") const { convertHBSBlock } = require("./conversion") +const javascript = require("./helpers/javascript") const hbsInstance = handlebars.create() registerAll(hbsInstance) @@ -362,6 +363,8 @@ module.exports.doesContainString = (template, string) => { return exports.doesContainStrings(template, [string]) } +module.exports.setJSRunner = javascript.setJSRunner + module.exports.convertToJS = hbs => { const blocks = exports.findHBSBlocks(hbs) let js = "return `", diff --git a/packages/string-templates/src/index.mjs b/packages/string-templates/src/index.mjs index ae640624b2..43cda8183f 100644 --- a/packages/string-templates/src/index.mjs +++ b/packages/string-templates/src/index.mjs @@ -1,6 +1,5 @@ import vm from "vm" import templates from "./index.js" -import { setJSRunner } from "./helpers/javascript" /** * ES6 entrypoint for rollup @@ -20,6 +19,7 @@ export const doesContainString = templates.doesContainString export const disableEscaping = templates.disableEscaping export const findHBSBlocks = templates.findHBSBlocks export const convertToJS = templates.convertToJS +export const setJSRunner = templates.setJSRunner export const FIND_ANY_HBS_REGEX = templates.FIND_ANY_HBS_REGEX if (process && !process.env.NO_JS) {