Merge branch 'master' into allow-calculation-fields-to-hide-required-fields

This commit is contained in:
Sam Rose 2024-10-08 09:30:00 +01:00 committed by GitHub
commit 1106244ff3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 541 additions and 49 deletions

View File

@ -184,6 +184,10 @@ spec:
- name: NODE_DEBUG
value: {{ .Values.services.apps.nodeDebug | quote }}
{{ end }}
{{ if .Values.services.apps.xssSafeMode }}
- name: XSS_SAFE_MODE
value: {{ .Values.services.apps.xssSafeMode | quote }}
{{ end }}
{{ if .Values.globals.datadogApmEnabled }}
- name: DD_LOGS_INJECTION
value: {{ .Values.globals.datadogApmEnabled | quote }}

View File

@ -1,6 +1,6 @@
{
"$schema": "node_modules/lerna/schemas/lerna-schema.json",
"version": "2.32.11",
"version": "2.32.12",
"npmClient": "yarn",
"packages": [
"packages/*",

View File

@ -571,6 +571,53 @@ describe.each([
expect(sum.calculationType).toEqual(CalculationType.SUM)
expect(sum.field).toEqual("Price")
})
it("cannot create a calculation view with more than 5 aggregations", async () => {
await config.api.viewV2.create(
{
tableId: table._id!,
name: generator.guid(),
schema: {
sum: {
visible: true,
calculationType: CalculationType.SUM,
field: "Price",
},
count: {
visible: true,
calculationType: CalculationType.COUNT,
field: "Price",
},
min: {
visible: true,
calculationType: CalculationType.MIN,
field: "Price",
},
max: {
visible: true,
calculationType: CalculationType.MAX,
field: "Price",
},
avg: {
visible: true,
calculationType: CalculationType.AVG,
field: "Price",
},
sum2: {
visible: true,
calculationType: CalculationType.SUM,
field: "Price",
},
},
},
{
status: 400,
body: {
message: "Calculation views can only have a maximum of 5 fields",
},
}
)
})
})
describe("update", () => {

View File

@ -83,6 +83,7 @@ const environment = {
PLUGINS_DIR: process.env.PLUGINS_DIR || DEFAULTS.PLUGINS_DIR,
MAX_IMPORT_SIZE_MB: process.env.MAX_IMPORT_SIZE_MB,
SESSION_EXPIRY_SECONDS: process.env.SESSION_EXPIRY_SECONDS,
XSS_SAFE_MODE: process.env.XSS_SAFE_MODE,
// SQL
SQL_MAX_ROWS: process.env.SQL_MAX_ROWS,
SQL_LOGGING_ENABLE: process.env.SQL_LOGGING_ENABLE,

View File

@ -8,7 +8,6 @@ import { init } from ".."
import TestConfiguration from "../../tests/utilities/TestConfiguration"
const DATE = "2021-01-21T12:00:00"
tk.freeze(DATE)
describe("jsRunner (using isolated-vm)", () => {
@ -48,6 +47,33 @@ describe("jsRunner (using isolated-vm)", () => {
).toEqual("ReferenceError: process is not defined")
})
it("should not allow the context to be mutated", async () => {
const context = { array: [1] }
const result = await processJS(
`
const array = $("array");
array.push(2);
return array[1]
`,
context
)
expect(result).toEqual(2)
expect(context.array).toEqual([1])
})
it("should copy values whenever returning them from $", async () => {
const context = { array: [1] }
const result = await processJS(
`
$("array").push(2);
return $("array")[1];
`,
context
)
expect(result).toEqual(undefined)
expect(context.array).toEqual([1])
})
describe("helpers", () => {
runJsHelpersTests({
funcWrap: (func: any) => config.doInContext(config.getAppId(), func),

View File

@ -8,6 +8,7 @@ import {
import { generateTableID } from "../../../../db/utils"
import { validate } from "../utils"
import { generator } from "@budibase/backend-core/tests"
import { withEnv } from "../../../../environment"
describe("validate", () => {
const hour = () => generator.hour().toString().padStart(2, "0")
@ -332,4 +333,46 @@ describe("validate", () => {
})
})
})
describe("XSS Safe mode", () => {
const getTable = (): Table => ({
type: "table",
_id: generateTableID(),
name: "table",
sourceId: INTERNAL_TABLE_SOURCE_ID,
sourceType: TableSourceType.INTERNAL,
schema: {
text: {
name: "sometext",
type: FieldType.STRING,
},
},
})
it.each([
"SELECT * FROM users WHERE username = 'admin' --",
"SELECT * FROM users WHERE id = 1; DROP TABLE users;",
"1' OR '1' = '1",
"' OR 'a' = 'a",
"<script>alert('XSS');</script>",
'"><img src=x onerror=alert(1)>',
"</script><script>alert('test')</script>",
"<div onmouseover=\"alert('XSS')\">Hover over me!</div>",
"'; EXEC sp_msforeachtable 'DROP TABLE ?'; --",
"{alert('Injected')}",
"UNION SELECT * FROM users",
"INSERT INTO users (username, password) VALUES ('admin', 'password')",
"/* This is a comment */ SELECT * FROM users",
'<iframe src="http://malicious-site.com"></iframe>',
])("test potentially unsafe input: %s", async input => {
await withEnv({ XSS_SAFE_MODE: "1" }, async () => {
const table = getTable()
const row = { text: input }
const output = await validate({ source: table, row })
expect(output.valid).toBe(false)
expect(output.errors).toStrictEqual({
text: ["Input not sanitised - potentially vulnerable to XSS"],
})
})
})
})
})

View File

@ -22,6 +22,7 @@ import { extractViewInfoFromID, isRelationshipColumn } from "../../../db/utils"
import { isSQL } from "../../../integrations/utils"
import { docIds, sql } from "@budibase/backend-core"
import { getTableFromSource } from "../../../api/controllers/row/utils"
import env from "../../../environment"
const SQL_CLIENT_SOURCE_MAP: Record<SourceName, SqlClient | undefined> = {
[SourceName.POSTGRES]: SqlClient.POSTGRES,
@ -43,6 +44,9 @@ const SQL_CLIENT_SOURCE_MAP: Record<SourceName, SqlClient | undefined> = {
[SourceName.BUDIBASE]: undefined,
}
const XSS_INPUT_REGEX =
/[<>;"'(){}]|--|\/\*|\*\/|union|select|insert|drop|delete|update|exec|script/i
export function getSQLClient(datasource: Datasource): SqlClient {
if (!isSQL(datasource)) {
throw new Error("Cannot get SQL Client for non-SQL datasource")
@ -222,6 +226,15 @@ export async function validate({
} else {
res = validateJs.single(row[fieldName], constraints)
}
if (env.XSS_SAFE_MODE && typeof row[fieldName] === "string") {
if (XSS_INPUT_REGEX.test(row[fieldName])) {
errors[fieldName] = [
"Input not sanitised - potentially vulnerable to XSS",
]
}
}
if (res) errors[fieldName] = res
}
return { valid: Object.keys(errors).length === 0, errors }

View File

@ -64,6 +64,14 @@ async function guardCalculationViewSchema(
view: Omit<ViewV2, "id" | "version">
) {
const calculationFields = helpers.views.calculationFields(view)
if (Object.keys(calculationFields).length > 5) {
throw new HTTPError(
"Calculation views can only have a maximum of 5 fields",
400
)
}
for (const calculationFieldName of Object.keys(calculationFields)) {
const schema = calculationFields[calculationFieldName]
const isCount = schema.calculationType === CalculationType.COUNT

View File

@ -1,13 +1,14 @@
import { atob, isJSAllowed } from "../utilities"
import cloneDeep from "lodash/fp/cloneDeep"
import { atob, isBackendService, isJSAllowed } from "../utilities"
import { LITERAL_MARKER } from "../helpers/constants"
import { getJsHelperList } from "./list"
import { iifeWrapper } from "../iife"
import { JsTimeoutError, UserScriptError } from "../errors"
import { cloneDeep } from "lodash/fp"
// The method of executing JS scripts depends on the bundle being built.
// This setter is used in the entrypoint (either index.js or index.mjs).
let runJS: ((js: string, context: any) => any) | undefined = undefined
let runJS: ((js: string, context: Record<string, any>) => any) | undefined =
undefined
export const setJSRunner = (runner: typeof runJS) => (runJS = runner)
export const removeJSRunner = () => {
@ -31,9 +32,19 @@ const removeSquareBrackets = (value: string) => {
return value
}
const isReservedKey = (key: string) =>
key === "snippets" ||
key === "helpers" ||
key.startsWith("snippets.") ||
key.startsWith("helpers.")
// Our context getter function provided to JS code as $.
// Extracts a value from context.
const getContextValue = (path: string, context: any) => {
// We populate `snippets` ourselves, don't allow access to it.
if (isReservedKey(path)) {
return undefined
}
const literalStringRegex = /^(["'`]).*\1$/
let data = context
// check if it's a literal string - just return path if its quoted
@ -46,6 +57,7 @@ const getContextValue = (path: string, context: any) => {
}
data = data[removeSquareBrackets(key)]
})
return data
}
@ -67,10 +79,23 @@ export function processJS(handlebars: string, context: any) {
snippetMap[snippet.name] = snippet.code
}
// Our $ context function gets a value from context.
// We clone the context to avoid mutation in the binding affecting real
// app context.
const clonedContext = cloneDeep({ ...context, snippets: null })
let clonedContext: Record<string, any>
if (isBackendService()) {
// On the backned, values are copied across the isolated-vm boundary and
// so we don't need to do any cloning here. This does create a fundamental
// difference in how JS executes on the frontend vs the backend, e.g.
// consider this snippet:
//
// $("array").push(2)
// return $("array")[1]
//
// With the context of `{ array: [1] }`, the backend will return
// `undefined` whereas the frontend will return `2`. We should fix this.
clonedContext = context
} else {
clonedContext = cloneDeep(context)
}
const sandboxContext = {
$: (path: string) => getContextValue(path, clonedContext),
helpers: getJsHelperList(),

View File

@ -1,4 +1,4 @@
import { Context, createContext, runInNewContext } from "vm"
import { createContext, runInNewContext } from "vm"
import { create, TemplateDelegate } from "handlebars"
import { registerAll, registerMinimum } from "./helpers/index"
import { postprocess, preprocess } from "./processors"
@ -455,21 +455,14 @@ export function convertToJS(hbs: string) {
export { JsTimeoutError, UserScriptError } from "./errors"
export function defaultJSSetup() {
if (!isBackendService()) {
/**
* Use polyfilled vm to run JS scripts in a browser Env
*/
setJSRunner((js: string, context: Context) => {
context = {
...context,
alert: undefined,
setInterval: undefined,
setTimeout: undefined,
}
createContext(context)
export function browserJSSetup() {
/**
* Use polyfilled vm to run JS scripts in a browser Env
*/
setJSRunner((js: string, context: Record<string, any>) => {
createContext(context)
const wrappedJs = `
const wrappedJs = `
result = {
result: null,
error: null,
@ -484,12 +477,17 @@ export function defaultJSSetup() {
result;
`
const result = runInNewContext(wrappedJs, context, { timeout: 1000 })
if (result.error) {
throw new UserScriptError(result.error)
}
return result.result
})
const result = runInNewContext(wrappedJs, context, { timeout: 1000 })
if (result.error) {
throw new UserScriptError(result.error)
}
return result.result
})
}
export function defaultJSSetup() {
if (!isBackendService()) {
browserJSSetup()
} else {
removeJSRunner()
}

View File

@ -4,7 +4,14 @@ export const FIND_HBS_REGEX = /{{([^{].*?)}}/g
export const FIND_ANY_HBS_REGEX = /{?{{([^{].*?)}}}?/g
export const FIND_TRIPLE_HBS_REGEX = /{{{([^{].*?)}}}/g
const isJest = () => typeof jest !== "undefined"
export const isBackendService = () => {
// We consider the tests for string-templates to be frontend, so that they
// test the frontend JS functionality.
if (isJest()) {
return false
}
return typeof window === "undefined"
}

View File

@ -1,7 +1,13 @@
import vm from "vm"
import { processStringSync, encodeJSBinding, setJSRunner } from "../src/index"
import {
processStringSync,
encodeJSBinding,
defaultJSSetup,
} from "../src/index"
import { UUID_REGEX } from "./constants"
import tk from "timekeeper"
const DATE = "2021-01-21T12:00:00"
tk.freeze(DATE)
const processJS = (js: string, context?: object): any => {
return processStringSync(encodeJSBinding(js), context)
@ -9,9 +15,7 @@ const processJS = (js: string, context?: object): any => {
describe("Javascript", () => {
beforeAll(() => {
setJSRunner((js, context) => {
return vm.runInNewContext(js, context, { timeout: 1000 })
})
defaultJSSetup()
})
describe("Test the JavaScript helper", () => {
@ -118,8 +122,7 @@ describe("Javascript", () => {
})
it("should handle errors", () => {
const output = processJS(`throw "Error"`)
expect(output).toBe("Error while executing JS")
expect(processJS(`throw "Error"`)).toEqual("Error")
})
it("should timeout after one second", () => {
@ -127,16 +130,18 @@ describe("Javascript", () => {
expect(output).toBe("Timed out 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 access to the process global", async () => {
expect(processJS(`return process`)).toEqual(
"ReferenceError: process is not defined"
)
})
})
describe("check JS helpers", () => {
it("should error if using the format helper. not helpers.", () => {
const output = processJS(`return helper.toInt(4.3)`)
expect(output).toBe("Error while executing JS")
expect(processJS(`return helper.toInt(4.3)`)).toEqual(
"ReferenceError: helper is not defined"
)
})
it("should be able to use toInt", () => {
@ -156,4 +161,323 @@ describe("Javascript", () => {
expect(output).toBe("Custom")
})
})
describe("mutability", () => {
it("should not allow the context to be mutated", async () => {
const context = { array: [1] }
const result = await processJS(
`
const array = $("array");
array.push(2);
return array[1]
`,
context
)
expect(result).toEqual(2)
expect(context.array).toEqual([1])
})
})
describe("malice", () => {
it("should not be able to call JS functions", () => {
expect(processJS(`return alert("hello")`)).toEqual(
"ReferenceError: alert is not defined"
)
expect(processJS(`return prompt("hello")`)).toEqual(
"ReferenceError: prompt is not defined"
)
expect(processJS(`return confirm("hello")`)).toEqual(
"ReferenceError: confirm is not defined"
)
expect(processJS(`return setTimeout(() => {}, 1000)`)).toEqual(
"ReferenceError: setTimeout is not defined"
)
expect(processJS(`return setInterval(() => {}, 1000)`)).toEqual(
"ReferenceError: setInterval is not defined"
)
})
})
// the test cases here were extracted from templates/real world examples of JS in Budibase
describe("real test cases from Budicloud", () => {
const context = {
"Unit Value": 2,
Quantity: 1,
}
it("handle test case 1", async () => {
const result = await processJS(
`
var Gross = $("[Unit Value]") * $("[Quantity]")
return Gross.toFixed(2)`,
context
)
expect(result).toBeDefined()
expect(result).toBe("2.00")
})
it("handle test case 2", async () => {
const todayDate = new Date()
// add a year and a month
todayDate.setMonth(new Date().getMonth() + 1)
todayDate.setFullYear(todayDate.getFullYear() + 1)
const context = {
"Purchase Date": DATE,
today: todayDate.toISOString(),
}
const result = await processJS(
`
var purchase = new Date($("[Purchase Date]"));
let purchaseyear = purchase.getFullYear();
let purchasemonth = purchase.getMonth();
var today = new Date($("today"));
let todayyear = today.getFullYear();
let todaymonth = today.getMonth();
var age = todayyear - purchaseyear
if (((todaymonth - purchasemonth) < 6) == true){
return age
}
`,
context
)
expect(result).toBeDefined()
expect(result).toBe(1)
})
it("should handle test case 3", async () => {
const context = {
Escalate: true,
"Budget ($)": 1100,
}
const result = await processJS(
`
if ($("[Escalate]") == true) {
if ($("Budget ($)") <= 1000)
{return 2;}
if ($("Budget ($)") > 1000)
{return 3;}
}
else {
if ($("Budget ($)") <= 1000)
{return 1;}
if ($("Budget ($)") > 1000)
if ($("Budget ($)") < 10000)
{return 2;}
else
{return 3}
}
`,
context
)
expect(result).toBeDefined()
expect(result).toBe(3)
})
it("should handle test case 4", async () => {
const context = {
"Time Sheets": ["a", "b"],
}
const result = await processJS(
`
let hours = 0
if (($("[Time Sheets]") != null) == true){
for (i = 0; i < $("[Time Sheets]").length; i++){
let hoursLogged = "Time Sheets." + i + ".Hours"
hours += $(hoursLogged)
}
return hours
}
if (($("[Time Sheets]") != null) == false){
return hours
}
`,
context
)
expect(result).toBeDefined()
expect(result).toBe("0ab")
})
it("should handle test case 5", async () => {
const context = {
change: JSON.stringify({ a: 1, primaryDisplay: "a" }),
previous: JSON.stringify({ a: 2, primaryDisplay: "b" }),
}
const result = await processJS(
`
let change = $("[change]") ? JSON.parse($("[change]")) : {}
let previous = $("[previous]") ? JSON.parse($("[previous]")) : {}
function simplifyLink(originalKey, value, parent) {
if (Array.isArray(value)) {
if (value.filter(item => Object.keys(item || {}).includes("primaryDisplay")).length > 0) {
parent[originalKey] = value.map(link => link.primaryDisplay)
}
}
}
for (let entry of Object.entries(change)) {
simplifyLink(entry[0], entry[1], change)
}
for (let entry of Object.entries(previous)) {
simplifyLink(entry[0], entry[1], previous)
}
let diff = Object.fromEntries(Object.entries(change).filter(([k, v]) => previous[k]?.toString() !== v?.toString()))
delete diff.audit_change
delete diff.audit_previous
delete diff._id
delete diff._rev
delete diff.tableId
delete diff.audit
for (let entry of Object.entries(diff)) {
simplifyLink(entry[0], entry[1], diff)
}
return JSON.stringify(change)?.replaceAll(",\\"", ",\\n\\t\\"").replaceAll("{\\"", "{\\n\\t\\"").replaceAll("}", "\\n}")
`,
context
)
expect(result).toBe(`{\n\t"a":1,\n\t"primaryDisplay":"a"\n}`)
})
it("should handle test case 6", async () => {
const context = {
"Join Date": DATE,
}
const result = await processJS(
`
var rate = 5;
var today = new Date();
// comment
function monthDiff(dateFrom, dateTo) {
return dateTo.getMonth() - dateFrom.getMonth() +
(12 * (dateTo.getFullYear() - dateFrom.getFullYear()))
}
var serviceMonths = monthDiff( new Date($("[Join Date]")), today);
var serviceYears = serviceMonths / 12;
if (serviceYears >= 1 && serviceYears < 5){
rate = 10;
}
if (serviceYears >= 5 && serviceYears < 10){
rate = 15;
}
if (serviceYears >= 10){
rate = 15;
rate += 0.5 * (Number(serviceYears.toFixed(0)) - 10);
}
return rate;
`,
context
)
expect(result).toBe(10)
})
it("should handle test case 7", async () => {
const context = {
"P I": "Pass",
"PA I": "Pass",
"F I": "Fail",
"V I": "Pass",
}
const result = await processJS(
`if (($("[P I]") == "Pass") == true)
if (($("[ P I]") == "Pass") == true)
if (($("[F I]") == "Pass") == true)
if (($("[V I]") == "Pass") == true)
{return "Pass"}
if (($("[PA I]") == "Fail") == true)
{return "Fail"}
if (($("[ P I]") == "Fail") == true)
{return "Fail"}
if (($("[F I]") == "Fail") == true)
{return "Fail"}
if (($("[V I]") == "Fail") == true)
{return "Fail"}
else
{return ""}`,
context
)
expect(result).toBe("Fail")
})
it("should handle test case 8", async () => {
const context = {
"T L": [{ Hours: 10 }],
"B H": 50,
}
const result = await processJS(
`var totalHours = 0;
if (($("[T L]") != null) == true){
for (let i = 0; i < ($("[T L]").length); i++){
var individualHours = "T L." + i + ".Hours";
var hoursNum = Number($(individualHours));
totalHours += hoursNum;
}
return totalHours.toFixed(2);
}
if (($("[T L]") != null) == false) {
return totalHours.toFixed(2);
}
`,
context
)
expect(result).toBe("10.00")
})
it("should handle test case 9", async () => {
const context = {
"T L": [{ Hours: 10 }],
"B H": 50,
}
const result = await processJS(
`var totalHours = 0;
if (($("[T L]") != null) == true){
for (let i = 0; i < ($("[T L]").length); i++){
var individualHours = "T L." + i + ".Hours";
var hoursNum = Number($(individualHours));
totalHours += hoursNum;
}
return ($("[B H]") - totalHours).toFixed(2);
}
if (($("[T L]") != null) == false) {
return ($("[B H]") - totalHours).toFixed(2);
}`,
context
)
expect(result).toBe("40.00")
})
it("should handle test case 10", async () => {
const context = {
"F F": [{ "F S": 10 }],
}
const result = await processJS(
`var rating = 0;
if ($("[F F]") != null){
for (i = 0; i < $("[F F]").length; i++){
var individualRating = $("F F." + i + ".F S");
rating += individualRating;
}
rating = (rating / $("[F F]").length);
}
return rating;
`,
context
)
expect(result).toBe(10)
})
})
})

View File

@ -1,5 +1,3 @@
import vm from "vm"
jest.mock("@budibase/handlebars-helpers/lib/math", () => {
const actual = jest.requireActual("@budibase/handlebars-helpers/lib/math")
@ -17,7 +15,7 @@ jest.mock("@budibase/handlebars-helpers/lib/uuid", () => {
}
})
import { processString, setJSRunner } from "../src/index"
import { defaultJSSetup, processString } from "../src/index"
import tk from "timekeeper"
import { getParsedManifest, runJsHelpersTests } from "./utils"
@ -32,9 +30,7 @@ describe("manifest", () => {
const manifest = getParsedManifest()
beforeAll(() => {
setJSRunner((js, context) => {
return vm.runInNewContext(js, context, { timeout: 1000 })
})
defaultJSSetup()
})
describe("examples are valid", () => {