diff --git a/charts/budibase/templates/app-service-deployment.yaml b/charts/budibase/templates/app-service-deployment.yaml index 2b099d01f5..4d0560312f 100644 --- a/charts/budibase/templates/app-service-deployment.yaml +++ b/charts/budibase/templates/app-service-deployment.yaml @@ -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 }} diff --git a/lerna.json b/lerna.json index a4bcb56d38..55721b6b2d 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.32.11", + "version": "2.32.12", "npmClient": "yarn", "packages": [ "packages/*", diff --git a/packages/backend-core/src/db/couch/DatabaseImpl.ts b/packages/backend-core/src/db/couch/DatabaseImpl.ts index 274c1b9e93..1213dde8f5 100644 --- a/packages/backend-core/src/db/couch/DatabaseImpl.ts +++ b/packages/backend-core/src/db/couch/DatabaseImpl.ts @@ -371,11 +371,21 @@ export class DatabaseImpl implements Database { return this.performCall(() => { return async () => { const response = await directCouchUrlCall(args) - const json = await response.json() + const text = await response.text() if (response.status > 300) { + let json + try { + json = JSON.parse(text) + } catch (err) { + console.error(`SQS error: ${text}`) + throw new CouchDBError( + "error while running SQS query, please try again later", + { name: "sqs_error", status: response.status } + ) + } throw json } - return json as T + return JSON.parse(text) as T } }) } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index b2b78853ef..e22f028caa 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -593,6 +593,53 @@ describe.each([ } ) }) + + 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", () => { @@ -1072,6 +1119,26 @@ describe.each([ ) }) + it("can add a new group by field that is invisible, even if required on the table", async () => { + view.schema!.name = { visible: false } + await config.api.viewV2.update(view) + + const { rows } = await config.api.row.search(view.id) + expect(rows).toHaveLength(2) + expect(rows).toEqual( + expect.arrayContaining([ + { + country: "USA", + age: 65, + }, + { + country: "UK", + age: 61, + }, + ]) + ) + }) + it("can add a new calculation field", async () => { view.schema!.count = { visible: true, diff --git a/packages/server/src/environment.ts b/packages/server/src/environment.ts index 585eb6a7f2..45d675ec3f 100644 --- a/packages/server/src/environment.ts +++ b/packages/server/src/environment.ts @@ -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, diff --git a/packages/server/src/sdk/app/rows/tests/utils.spec.ts b/packages/server/src/sdk/app/rows/tests/utils.spec.ts index 548b2b6bc9..516334d31d 100644 --- a/packages/server/src/sdk/app/rows/tests/utils.spec.ts +++ b/packages/server/src/sdk/app/rows/tests/utils.spec.ts @@ -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", + "", + '">', + "", + "
Hover over me!
", + "'; 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", + '', + ])("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"], + }) + }) + }) + }) }) diff --git a/packages/server/src/sdk/app/rows/utils.ts b/packages/server/src/sdk/app/rows/utils.ts index 6ef4dcbc8e..bded6a7a18 100644 --- a/packages/server/src/sdk/app/rows/utils.ts +++ b/packages/server/src/sdk/app/rows/utils.ts @@ -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.POSTGRES]: SqlClient.POSTGRES, @@ -43,6 +44,9 @@ const SQL_CLIENT_SOURCE_MAP: Record = { [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 } diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index e217f16400..fc198bf2b9 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -64,6 +64,14 @@ async function guardCalculationViewSchema( view: Omit ) { 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 @@ -121,7 +129,11 @@ async function guardViewSchema( } await checkReadonlyFields(table, view) - checkRequiredFields(table, view) + + if (!helpers.views.isCalculationView(view)) { + checkRequiredFields(table, view) + } + checkDisplayField(view) }