From b4cb97963c60a857c58e195ef47d43d668102af6 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 21 Nov 2023 10:40:25 +0000 Subject: [PATCH 1/5] Move from an allow list to a block list of file extensions. --- .../src/api/controllers/static/index.ts | 7 +- packages/shared-core/src/constants.ts | 92 ++++++++++--------- 2 files changed, 56 insertions(+), 43 deletions(-) diff --git a/packages/server/src/api/controllers/static/index.ts b/packages/server/src/api/controllers/static/index.ts index 544bde1fbb..4c5415a6c6 100644 --- a/packages/server/src/api/controllers/static/index.ts +++ b/packages/server/src/api/controllers/static/index.ts @@ -1,4 +1,4 @@ -import { ValidFileExtensions } from "@budibase/shared-core" +import { InvalidFileExtensions } from "@budibase/shared-core" require("svelte/register") @@ -86,7 +86,10 @@ export const uploadFile = async function ( ) } - if (!env.SELF_HOSTED && !ValidFileExtensions.includes(extension)) { + if ( + !env.SELF_HOSTED && + InvalidFileExtensions.includes(extension.toLowerCase()) + ) { throw new BadRequestError( `File "${file.name}" has an invalid extension: "${extension}"` ) diff --git a/packages/shared-core/src/constants.ts b/packages/shared-core/src/constants.ts index e7c6feb20a..5b50d0eb3b 100644 --- a/packages/shared-core/src/constants.ts +++ b/packages/shared-core/src/constants.ts @@ -96,45 +96,55 @@ export enum BuilderSocketEvent { export const SocketSessionTTL = 60 export const ValidQueryNameRegex = /^[^()]*$/ export const ValidColumnNameRegex = /^[_a-zA-Z0-9\s]*$/g -export const ValidFileExtensions = [ - "avif", - "css", - "csv", - "docx", - "drawio", - "editorconfig", - "edl", - "enc", - "export", - "geojson", - "gif", - "htm", - "html", - "ics", - "iqy", - "jfif", - "jpeg", - "jpg", - "json", - "log", - "md", - "mid", - "odt", - "pdf", - "png", - "ris", - "rtf", - "svg", - "tex", - "toml", - "twig", - "txt", - "url", - "wav", - "webp", - "xls", - "xlsx", - "xml", - "yaml", - "yml", + +export const InvalidFileExtensions = [ + "action", + "apk", + "app", + "bat", + "bin", + "cab", + "cmd", + "com", + "command", + "cpl", + "csh", + "ex_", + "exe", + "gadget", + "inf1", + "ins", + "inx", + "ipa", + "isu", + "job", + "jse", + "ksh", + "lnk", + "msc", + "msi", + "msp", + "mst", + "osx", + "out", + "paf", + "pif", + "prg", + "ps1", + "reg", + "rgs", + "run", + "scr", + "sct", + "shb", + "shs", + "u3p", + "vb", + "vbe", + "vbs", + "vbscript", + "workflow", + "ws", + "wsf", + "wsh", ] From 79dcc468b899a9caf922f74ef17316315ea6592f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 21 Nov 2023 10:42:44 +0000 Subject: [PATCH 2/5] Add a test for uppercase malicious extensions. --- .../server/src/api/routes/tests/attachment.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/server/src/api/routes/tests/attachment.spec.ts b/packages/server/src/api/routes/tests/attachment.spec.ts index 14d2e845f6..e230b0688a 100644 --- a/packages/server/src/api/routes/tests/attachment.spec.ts +++ b/packages/server/src/api/routes/tests/attachment.spec.ts @@ -35,6 +35,17 @@ describe("/api/applications/:appId/sync", () => { }) }) + it("should reject an upload with a malicious uppercase file extension", async () => { + await config.withEnv({ SELF_HOSTED: undefined }, async () => { + let resp = (await config.api.attachment.process( + "OHNO.EXE", + Buffer.from([0]), + { expectStatus: 400 } + )) as unknown as APIError + expect(resp.message).toContain("invalid extension") + }) + }) + it("should reject an upload with no file", async () => { let resp = (await config.api.attachment.process( undefined as any, From 0df315c4780a24a1dc98864a5649bcf0e3d28c58 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 21 Nov 2023 10:52:50 +0000 Subject: [PATCH 3/5] There has been quite a few redlock errors the last while - the message does not provide much information about what is going wrong - trying to rectify this to see if this is an error that could be impacting performance. --- packages/backend-core/src/redis/redlockImpl.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/redis/redlockImpl.ts b/packages/backend-core/src/redis/redlockImpl.ts index 7fe61a409e..266f1fe989 100644 --- a/packages/backend-core/src/redis/redlockImpl.ts +++ b/packages/backend-core/src/redis/redlockImpl.ts @@ -3,6 +3,7 @@ import { getLockClient } from "./init" import { LockOptions, LockType } from "@budibase/types" import * as context from "../context" import env from "../environment" +import { logWarn } from "../logging" async function getClient( type: LockType, @@ -116,7 +117,7 @@ export async function doWithLock( const result = await task() return { executed: true, result } } catch (e: any) { - console.warn("lock error") + logWarn(`lock type: ${opts.type} error`, e) // lock limit exceeded if (e.name === "LockError") { if (opts.type === LockType.TRY_ONCE) { @@ -124,11 +125,9 @@ export async function doWithLock( // due to retry count (0) exceeded return { executed: false } } else { - console.error(e) throw e } } else { - console.error(e) throw e } } finally { From 71b8b300bf4af9a91a0ab4558dace7462b3134a9 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 21 Nov 2023 11:26:25 +0000 Subject: [PATCH 4/5] Add archive file formats to blocklist. --- packages/shared-core/src/constants.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/shared-core/src/constants.ts b/packages/shared-core/src/constants.ts index 5b50d0eb3b..3838cb7636 100644 --- a/packages/shared-core/src/constants.ts +++ b/packages/shared-core/src/constants.ts @@ -98,6 +98,7 @@ export const ValidQueryNameRegex = /^[^()]*$/ export const ValidColumnNameRegex = /^[_a-zA-Z0-9\s]*$/g export const InvalidFileExtensions = [ + "7z", "action", "apk", "app", @@ -138,6 +139,7 @@ export const InvalidFileExtensions = [ "sct", "shb", "shs", + "tar", "u3p", "vb", "vbe", @@ -147,4 +149,5 @@ export const InvalidFileExtensions = [ "ws", "wsf", "wsh", + "zip", ] From 303ae3c58b0f8e646b7ac40da547ec1a03db3552 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 21 Nov 2023 11:41:32 +0000 Subject: [PATCH 5/5] Block some code extensions that could be used maliciously. --- packages/shared-core/src/constants.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/shared-core/src/constants.ts b/packages/shared-core/src/constants.ts index 3838cb7636..0787b8bed1 100644 --- a/packages/shared-core/src/constants.ts +++ b/packages/shared-core/src/constants.ts @@ -119,6 +119,7 @@ export const InvalidFileExtensions = [ "ipa", "isu", "job", + "js", "jse", "ksh", "lnk", @@ -129,6 +130,7 @@ export const InvalidFileExtensions = [ "osx", "out", "paf", + "php", "pif", "prg", "ps1", @@ -145,6 +147,7 @@ export const InvalidFileExtensions = [ "vbe", "vbs", "vbscript", + "wasm", "workflow", "ws", "wsf",