From 68bc7bbdf53f32a540bb4ce5bf574d9d9df6c066 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 19 Mar 2024 12:52:06 +0000 Subject: [PATCH 1/3] Fixing issue, dis-allow passing HBS statements in as query parameters. --- .../server/src/api/controllers/query/index.ts | 26 ++++++++++++++----- packages/types/src/api/web/query.ts | 2 +- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index 3c21537484..234508ef32 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -14,22 +14,36 @@ import { SessionCookie, JsonFieldSubType, QueryResponse, - QueryPreview, QuerySchema, FieldType, ExecuteQueryRequest, ExecuteQueryResponse, - Row, QueryParameter, PreviewQueryRequest, PreviewQueryResponse, } from "@budibase/types" import { ValidQueryNameRegex, utils as JsonUtils } from "@budibase/shared-core" +import { findHBSBlocks } from "@budibase/string-templates" const Runner = new Thread(ThreadType.QUERY, { timeoutMs: env.QUERY_THREAD_TIMEOUT, }) +function validateQueryInputs(parameters: Record) { + for (let entry of Object.entries(parameters)) { + const key = entry[0], + value = entry[1] + if (typeof value !== "string") { + continue + } + if (findHBSBlocks(value).length !== 0) { + throw new Error( + `Parameter '${key}' input contains a handlebars binding - this is not allowed.` + ) + } + } +} + export async function fetch(ctx: UserCtx) { ctx.body = await sdk.queries.fetch() } @@ -123,10 +137,10 @@ function getAuthConfig(ctx: UserCtx) { function enrichParameters( queryParameters: QueryParameter[], - requestParameters: { [key: string]: string } = {} -): { - [key: string]: string -} { + requestParameters: Record = {} +): Record { + // first check parameters are all valid + validateQueryInputs(requestParameters) // make sure parameters are fully enriched with defaults for (let parameter of queryParameters) { if (!requestParameters[parameter.name]) { diff --git a/packages/types/src/api/web/query.ts b/packages/types/src/api/web/query.ts index 3959cdea19..66f0248647 100644 --- a/packages/types/src/api/web/query.ts +++ b/packages/types/src/api/web/query.ts @@ -11,7 +11,7 @@ export interface PreviewQueryResponse { } export interface ExecuteQueryRequest { - parameters?: { [key: string]: string } + parameters?: Record pagination?: any } From abdff7d8e68e8d88a406ce2b8a9f8d236a7769c1 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 19 Mar 2024 13:22:43 +0000 Subject: [PATCH 2/3] Adding test case. --- .../api/routes/tests/queries/query.seq.spec.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/server/src/api/routes/tests/queries/query.seq.spec.ts b/packages/server/src/api/routes/tests/queries/query.seq.spec.ts index 4347ed9044..fc90ad4247 100644 --- a/packages/server/src/api/routes/tests/queries/query.seq.spec.ts +++ b/packages/server/src/api/routes/tests/queries/query.seq.spec.ts @@ -408,6 +408,21 @@ describe("/queries", () => { }, }) }) + + it("shouldn't allow handlebars to be passed as parameters", async () => { + const res = await request + .post(`/api/queries/${query._id}`) + .send({ + parameters: { + a: "{{ 'test' }}", + }, + }) + .set(config.defaultHeaders()) + .expect(400) + expect(res.body.message).toEqual( + "Parameter 'a' input contains a handlebars binding - this is not allowed." + ) + }) }) describe("variables", () => { From 55b9b00771935320a70691bd0005c708e451c0b8 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 19 Mar 2024 14:50:38 +0000 Subject: [PATCH 3/3] PR comments. --- packages/server/src/api/controllers/query/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index 234508ef32..0dba20dacd 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -31,8 +31,7 @@ const Runner = new Thread(ThreadType.QUERY, { function validateQueryInputs(parameters: Record) { for (let entry of Object.entries(parameters)) { - const key = entry[0], - value = entry[1] + const [key, value] = entry if (typeof value !== "string") { continue }