From 184acb279f3c7afcb3ed7c815010494ae92157da Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:57:43 +0800 Subject: [PATCH 01/26] [MOL-22453][SX][asgard-0001] Fix CI script injection in trigger-gitlab-pipeline.yml Route github.head_ref, github.event.pull_request.title, and github.event.head_commit.message through env: instead of inline ${{ }} interpolation in the run: block. ${{ }} expansion happens before bash executes the script, so an attacker-controlled PR title or branch name was previously substituted as literal script text and executed as code. Mirrors the safe pattern already used one step later in the same file for GITLAB_TOKEN/GITLAB_PAT. --- .github/workflows/trigger-gitlab-pipeline.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/trigger-gitlab-pipeline.yml b/.github/workflows/trigger-gitlab-pipeline.yml index babc714f3..fd7f0d83d 100644 --- a/.github/workflows/trigger-gitlab-pipeline.yml +++ b/.github/workflows/trigger-gitlab-pipeline.yml @@ -16,10 +16,15 @@ jobs: steps: - name: Print Configs + env: + HEAD_REF: ${{ github.head_ref }} + REF_NAME: ${{ github.ref_name }} + PR_TITLE: ${{ github.event.pull_request.title }} + HEAD_COMMIT_MSG: ${{ github.event.head_commit.message }} run: | - [[ $GITHUB_EVENT_NAME = "pull_request" ]] && BRANCH_NAME="${{ github.head_ref }}" || BRANCH_NAME="${{ github.ref_name }}" + [[ $GITHUB_EVENT_NAME = "pull_request" ]] && BRANCH_NAME="$HEAD_REF" || BRANCH_NAME="$REF_NAME" - [[ $GITHUB_EVENT_NAME = "pull_request" ]] && COMMIT_MSG="${{ github.event.pull_request.title }}" || COMMIT_MSG=$(echo -e "${{ github.event.head_commit.message }}" | head -n 1) + [[ $GITHUB_EVENT_NAME = "pull_request" ]] && COMMIT_MSG="$PR_TITLE" || COMMIT_MSG=$(echo -e "$HEAD_COMMIT_MSG" | head -n 1) PIPELINE_PROJECT_URL="github.com/$GITHUB_REPOSITORY.git" From 3e87a1c1c6f791d3fdf4f88625bb428abc3dec43 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:57:51 +0800 Subject: [PATCH 02/26] [MOL-22453][SX][asgard-0002] Fix sanitize-html allowedAttributes:false misuse in Typography MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit allowedAttributes: false means "allow ALL attributes on all tags", not "allow none" — the opposite of the intended restriction. This let sanitized rich text carry arbitrary attributes (e.g. event handlers, style) straight through. Replace with an explicit allowlist built on sanitize-html's own defaults plus the specific img attributes this component actually needs. --- .../elements/typography/typography.spec.tsx | 12 ++++++++++++ src/components/elements/typography/typography.tsx | 5 ++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/__tests__/components/elements/typography/typography.spec.tsx b/src/__tests__/components/elements/typography/typography.spec.tsx index f7f0e1658..6f29eb9ea 100644 --- a/src/__tests__/components/elements/typography/typography.spec.tsx +++ b/src/__tests__/components/elements/typography/typography.spec.tsx @@ -109,6 +109,18 @@ describe(UI_TYPE, () => { expect(screen.getByText("This is a HTML string")).toBeInTheDocument(); }); + it("should strip event handler attributes from an otherwise-allowed image tag", () => { + renderComponent({ + className: "text-element", + children: '\'broken', + }); + + const imgElement = screen.getByAltText("broken image"); + expect(imgElement).toBeInTheDocument(); + expect(imgElement).not.toHaveAttribute("onerror"); + expect(document.querySelector(".text-element").innerHTML).not.toContain("onerror"); + }); + it("should be able to sanitize HTML string", () => { renderComponent({ className: "text-element", diff --git a/src/components/elements/typography/typography.tsx b/src/components/elements/typography/typography.tsx index 481d0f90a..e937f4f3d 100644 --- a/src/components/elements/typography/typography.tsx +++ b/src/components/elements/typography/typography.tsx @@ -61,7 +61,10 @@ export const Typography = (props: IGenericElementProps) => { // ============================================================================= const sanitizeOptions: IOptions = { allowedTags: sanitizeHtml.defaults.allowedTags.concat(["img"]), - allowedAttributes: false, + allowedAttributes: { + ...sanitizeHtml.defaults.allowedAttributes, + img: ["src", "alt", "width", "height"], + }, }; const renderText = (): JSX.Element[] | JSX.Element | string[] | string => { From d061dffa7b0fffec4fce7324c4257b5b52c20704 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:58:03 +0800 Subject: [PATCH 03/26] [MOL-22453][SX][asgard-0003] Fix sanitize-html allowedAttributes:false misuse in FilterCheckbox Same allowedAttributes: false misconfiguration as asgard-0002 (means "allow all attributes", not "allow none"). Replace with sanitize-html's own default allowlist for this label-rendering sink. --- .../components/custom/filter/filter-checkbox.spec.tsx | 9 +++++++++ .../custom/filter/filter-checkbox/filter-checkbox.tsx | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/__tests__/components/custom/filter/filter-checkbox.spec.tsx b/src/__tests__/components/custom/filter/filter-checkbox.spec.tsx index 41dc58c74..75e1c7081 100644 --- a/src/__tests__/components/custom/filter/filter-checkbox.spec.tsx +++ b/src/__tests__/components/custom/filter/filter-checkbox.spec.tsx @@ -85,6 +85,15 @@ describe(REFERENCE_KEY, () => { expect(SUBMIT_FN).toHaveBeenCalledWith(expect.objectContaining({ [COMPONENT_ID]: defaultValues })); }); + it("should strip event handler attributes from option labels", () => { + renderComponent({ + options: [{ label: 'Apple label', value: "Apple" }], + }); + + const spanElement = screen.getByText("Apple label"); + expect(spanElement).not.toHaveAttribute("onclick"); + }); + it("should be able to render hint", () => { renderComponent({ label: { diff --git a/src/components/custom/filter/filter-checkbox/filter-checkbox.tsx b/src/components/custom/filter/filter-checkbox/filter-checkbox.tsx index 8ea642ba9..138e59864 100644 --- a/src/components/custom/filter/filter-checkbox/filter-checkbox.tsx +++ b/src/components/custom/filter/filter-checkbox/filter-checkbox.tsx @@ -3,6 +3,7 @@ import { useEffect, useState } from "react"; import { useFormContext } from "react-hook-form"; import useDeepCompareEffect from "use-deep-compare-effect"; import * as Yup from "yup"; +import sanitizeHtml from "sanitize-html"; import { TestHelper, filterSchemaProps } from "../../../../utils"; import { useValidationConfig } from "../../../../utils/hooks"; import { Sanitize } from "../../../shared"; @@ -89,7 +90,7 @@ export const FilterCheckbox = (props: IGenericCustomFieldProps (isParentOption(item) ? item.key : item.value)} labelExtractor={(item) => ( - + {item.label} )} From f84e7662c551613bc08106976a4815bb35edcd9d Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:58:16 +0800 Subject: [PATCH 04/26] [MOL-22453][SX][asgard-0006] Fix sanitize-html allowedAttributes:false misuse in Popover Same allowedAttributes: false misconfiguration as asgard-0002/0003 (means "allow all attributes", not "allow none"). Replace with an explicit allowlist built on sanitize-html's own defaults plus the specific img attributes this component's hint content needs. --- .../components/elements/popover/popover.spec.tsx | 12 ++++++++++++ src/components/elements/popover/popover.tsx | 5 ++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/__tests__/components/elements/popover/popover.spec.tsx b/src/__tests__/components/elements/popover/popover.spec.tsx index f4f19d540..c1038e138 100644 --- a/src/__tests__/components/elements/popover/popover.spec.tsx +++ b/src/__tests__/components/elements/popover/popover.spec.tsx @@ -69,6 +69,18 @@ describe(UI_TYPE, () => { expect(screen.queryByTestId("popover").innerHTML.includes("script")).toBe(false); }); + it("should strip event handler attributes from an otherwise-allowed image tag in the hint", () => { + renderComponent({ + hint: { content: '\'broken' }, + }); + + fireEvent.click(screen.getByTestId("field__popover")); + + const imgElement = screen.getByAltText("broken image"); + expect(imgElement).toBeInTheDocument(); + expect(imgElement).not.toHaveAttribute("onerror"); + }); + it("should render icon after text if specified", async () => { renderComponent({ icon: "AlbumFillIcon" }); diff --git a/src/components/elements/popover/popover.tsx b/src/components/elements/popover/popover.tsx index 789583d1f..9a3255d02 100644 --- a/src/components/elements/popover/popover.tsx +++ b/src/components/elements/popover/popover.tsx @@ -35,7 +35,10 @@ export const Popover = (props: IGenericElementProps) => { const renderPopoverContent = () => { const sanitizeOptions: IOptions = { allowedTags: sanitizeHtml.defaults.allowedTags.concat(["img"]), - allowedAttributes: false, + allowedAttributes: { + ...sanitizeHtml.defaults.allowedAttributes, + img: ["src", "alt", "width", "height"], + }, }; if (typeof hintContent === "string") { return ( From 7fca2d66a06963fb9d95ad831ad924beb6cc044e Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:58:29 +0800 Subject: [PATCH 05/26] [MOL-22453][SX][asgard-0005] Restrict ButtonField link URLs to a scheme allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isValidUrl only checked that the URL parsed, not its scheme — javascript:, data:, and vbscript: URLs all parse successfully and were treated as "valid", letting a schema-configured button link execute arbitrary script on click. Restrict to http:, https:, mailto:, and tel: — the only schemes this field is meant to support. --- src/__tests__/components/fields/button/button.spec.tsx | 8 +++++--- src/components/fields/button/button.tsx | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/__tests__/components/fields/button/button.spec.tsx b/src/__tests__/components/fields/button/button.spec.tsx index 8a1ab3031..e8a4f656c 100644 --- a/src/__tests__/components/fields/button/button.spec.tsx +++ b/src/__tests__/components/fields/button/button.spec.tsx @@ -125,9 +125,11 @@ describe("button", () => { }); it.each` - scenario | href - ${"should not navigate when href is not provided"} | ${undefined} - ${"should not navigate when href is invalid"} | ${"invalid-url"} + scenario | href + ${"should not navigate when href is not provided"} | ${undefined} + ${"should not navigate when href is invalid"} | ${"invalid-url"} + ${"should not navigate when href uses javascript: scheme"} | ${"javascript:alert(1)"} + ${"should not navigate when href uses data: scheme"} | ${"data:text/html,"} `("$scenario", ({ href }) => { renderComponent({ overrideButton: { ...(href && { href }) } }); fireEvent.click(getField("button", COMPONENT_LABEL)); diff --git a/src/components/fields/button/button.tsx b/src/components/fields/button/button.tsx index 2c2a5d3c8..b13f088f8 100644 --- a/src/components/fields/button/button.tsx +++ b/src/components/fields/button/button.tsx @@ -5,6 +5,8 @@ import { useFieldEvent } from "../../../utils/hooks"; import { filterSchemaProps } from "../../../utils/prop-helper"; import { IButtonSchema } from "./types"; +const ALLOWED_URL_SCHEMES = ["http:", "https:", "mailto:", "tel:"]; + export const ButtonField = (props: IGenericFieldProps) => { // ============================================================================= // CONST, STATE, REF @@ -45,7 +47,7 @@ export const ButtonField = (props: IGenericFieldProps) => { const isValidUrl = (url: string): boolean => { try { - return !!new URL(url); + return ALLOWED_URL_SCHEMES.includes(new URL(url).protocol); } catch { return false; } From 4b2a94769f522a54f10b09aea66f359144a8619f Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:58:43 +0800 Subject: [PATCH 06/26] [MOL-22453][SX][asgard-0007] Enforce postMessage origin check on Iframe field (hard default-on) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useIframeMessage accepted postMessage events from any origin, so any page/frame on the client could forge SYNC/RESIZE/CHANGE/VALIDATION/ LOADED events into the parent form. Derive the expected origin from the iframe's own configured src and reject any message whose event.origin doesn't match. event.origin is set by the browser from the actual sender and cannot be spoofed by script. Enforced unconditionally (no opt-in schema flag) — every consumer gets this protection by default. --- .../components/custom/iframe/iframe.spec.tsx | 24 +++++++++++++++++++ src/components/custom/iframe/iframe.tsx | 20 ++++++++++++---- src/utils/hooks/use-iframe-message.ts | 5 ++-- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/__tests__/components/custom/iframe/iframe.spec.tsx b/src/__tests__/components/custom/iframe/iframe.spec.tsx index 0f6567afd..559252e91 100644 --- a/src/__tests__/components/custom/iframe/iframe.spec.tsx +++ b/src/__tests__/components/custom/iframe/iframe.spec.tsx @@ -130,6 +130,30 @@ describe("iframe", () => { expect(SUBMIT_FN).toHaveBeenCalledWith(expect.objectContaining({ [COMPONENT_ID]: "hello world" })); }); + describe("postMessage origin validation", () => { + const sendPostMessageFromOrigin = (origin: string, type: EPostMessageEvent, payload?: unknown) => { + fireEvent(window, new MessageEvent("message", { data: { type, payload }, origin })); + }; + + it("should ignore a setValue postMessage from an origin that does not match src", async () => { + renderComponent({ validationTimeout: -1 }); + + sendPostMessageFromOrigin("https://attacker.example", EPostMessageEvent.SET_VALUE, "hello world"); + await waitFor(() => fireEvent.click(getSubmitButton())); + + expect(SUBMIT_FN).toHaveBeenCalledWith({}); + }); + + it("should accept a setValue postMessage from the origin matching src", async () => { + renderComponent({ validationTimeout: -1 }); + + sendPostMessageFromOrigin(IFRAME_SRC, EPostMessageEvent.SET_VALUE, "hello world"); + await waitFor(() => fireEvent.click(getSubmitButton())); + + expect(SUBMIT_FN).toHaveBeenCalledWith(expect.objectContaining({ [COMPONENT_ID]: "hello world" })); + }); + }); + describe("load", () => { it("should fire a loading event when iframe starts loading", () => { const testFn = jest.fn(); diff --git a/src/components/custom/iframe/iframe.tsx b/src/components/custom/iframe/iframe.tsx index d26eccbf8..b8a65cacb 100644 --- a/src/components/custom/iframe/iframe.tsx +++ b/src/components/custom/iframe/iframe.tsx @@ -130,11 +130,17 @@ export const Iframe = (props: IGenericCustomFieldProps) => { // ========================================================================= // POSTMESSAGE HANDLERS // ========================================================================= + // only messages from the origin derived from `src` are accepted; a child iframe that + // navigates to a different origin before posting back will need to do so via the + // original src origin's window, or the message is dropped + const allowedOrigin = getTargetOriginFromSrc(); + useIframeMessage( EPostMessageEvent.TRIGGER_SYNC, useCallback(() => { iframePostMessage({ type: EPostMessageEvent.SYNC, payload: { error, id, value } }); - }, [error, id, value, iframePostMessage]) + }, [error, id, value, iframePostMessage]), + allowedOrigin ); useIframeMessage<{ width?: number | undefined; height?: number | undefined }>( @@ -144,7 +150,8 @@ export const Iframe = (props: IGenericCustomFieldProps) => { width: e.data.payload?.width, height: e.data.payload?.height, }); - }, []) + }, []), + allowedOrigin ); useIframeMessage( @@ -154,7 +161,8 @@ export const Iframe = (props: IGenericCustomFieldProps) => { formContext.setValue(id, e.data.payload, { shouldDirty: true }); }, [formContext, id] - ) + ), + allowedOrigin ); useIframeMessage( @@ -169,14 +177,16 @@ export const Iframe = (props: IGenericCustomFieldProps) => { clearAsyncValidation(); }, [clearAsyncValidation] - ) + ), + allowedOrigin ); useIframeMessage( EPostMessageEvent.LOADED, useCallback(() => { dispatchFieldEvent("loaded", id); - }, [dispatchFieldEvent, id]) + }, [dispatchFieldEvent, id]), + allowedOrigin ); // ========================================================================= diff --git a/src/utils/hooks/use-iframe-message.ts b/src/utils/hooks/use-iframe-message.ts index 13d673e6a..1d1f36049 100644 --- a/src/utils/hooks/use-iframe-message.ts +++ b/src/utils/hooks/use-iframe-message.ts @@ -2,9 +2,10 @@ import { useEffect } from "react"; type MessageHandler = (event: MessageEvent<{ payload: T }>) => void; -export const useIframeMessage = (eventType: string, handler: MessageHandler) => { +export const useIframeMessage = (eventType: string, handler: MessageHandler, allowedOrigin?: string) => { useEffect(() => { const eventHandler = (event: MessageEvent) => { + if (allowedOrigin && event.origin !== allowedOrigin) return; if (event.data.type === eventType) { handler(event); } @@ -17,5 +18,5 @@ export const useIframeMessage = (eventType: string, handler: MessageHandler { window.removeEventListener("message", eventHandler); }; - }, [eventType, handler]); + }, [eventType, handler, allowedOrigin]); }; From d24f592a932529622f02762b9d99d453167619af Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:58:56 +0800 Subject: [PATCH 07/26] [MOL-22453][SX][asgard-0008] Escape user search query before compiling into RegExp boldResultsWithQuery compiled the raw user-typed search query directly into a RegExp used to highlight matches. An unescaped query containing regex metacharacters could throw (invalid pattern) or, with a crafted pattern, cause catastrophic backtracking (ReDoS) against the address list being rendered. Escape regex metacharacters before compiling and fail safe (return the list unhighlighted) if construction still throws. --- .../location-search-helper.spec.ts | 31 +++++++++++++++++++ .../location-modal/location-search/helper.ts | 9 +++++- 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/components/fields/location-field/location-search-helper.spec.ts diff --git a/src/__tests__/components/fields/location-field/location-search-helper.spec.ts b/src/__tests__/components/fields/location-field/location-search-helper.spec.ts new file mode 100644 index 000000000..14b9e93a2 --- /dev/null +++ b/src/__tests__/components/fields/location-field/location-search-helper.spec.ts @@ -0,0 +1,31 @@ +import { boldResultsWithQuery } from "../../../../components/fields/location-field/location-modal/location-search/helper"; +import { IResultListItem } from "../../../../components/fields/location-field/types"; + +const buildResult = (address: string): IResultListItem => ({ + address, + displayAddressText: undefined, +}); + +describe("boldResultsWithQuery", () => { + it("should bold the matching portion of the address", () => { + const [result] = boldResultsWithQuery([buildResult("123 Example Street")], "Example"); + + expect(result.displayAddressText).toBe('123 Example Street'); + }); + + it("should treat regex metacharacters in the query as literal characters", () => { + const [result] = boldResultsWithQuery([buildResult("Blk 5 (Example)")], "(Example)"); + + expect(result.displayAddressText).toBe('Blk 5 (Example)'); + }); + + it("should not hang or throw on a pathological regex-injection query", () => { + const maliciousQuery = "(a+)+$"; + + const start = Date.now(); + expect(() => + boldResultsWithQuery([buildResult("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!")], maliciousQuery) + ).not.toThrow(); + expect(Date.now() - start).toBeLessThan(1000); + }); +}); diff --git a/src/components/fields/location-field/location-modal/location-search/helper.ts b/src/components/fields/location-field/location-modal/location-search/helper.ts index 78c26275c..98031c93b 100644 --- a/src/components/fields/location-field/location-modal/location-search/helper.ts +++ b/src/components/fields/location-field/location-modal/location-search/helper.ts @@ -4,8 +4,15 @@ export const pagination = (array: T[], pageSize: number, pageNum: number) => return array.slice((pageNum - 1) * pageSize, pageNum * pageSize); }; +const escapeRegExp = (value: string): string => value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + export const boldResultsWithQuery = (arr: IResultListItem[], query: string) => { - const regex = new RegExp(query, "gi"); + let regex: RegExp; + try { + regex = new RegExp(escapeRegExp(query), "gi"); + } catch { + return arr; + } return arr.map((obj) => { const newAddress = (obj.displayAddressText || obj.address).replace( regex, From 6792ebb9108b4281bb294e61f2a30987542e4b3f Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:59:10 +0800 Subject: [PATCH 08/26] [MOL-22453][SX] Bump sanitize-html 2.17.5 -> 2.17.7 Picks up upstream fixes for known sanitize-html CVEs identified by npm audit. Declared range (^2.8.1 -> ^2.17.7) still allows future non-breaking patch/minor updates. --- package-lock.json | 121 ++++++++++++++++++++++++++++++++++++++++++++-- package.json | 2 +- 2 files changed, 117 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index bed20d136..7c8606c49 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30,7 +30,7 @@ "react-dropzone": "^14.2.3", "react-hook-form": "7.54.2", "react-infinite-scroll-hook": "^4.1.1", - "sanitize-html": "^2.8.1", + "sanitize-html": "^2.17.7", "use-deep-compare-effect": "^1.8.1", "yup": "^0.32.11" }, @@ -9789,6 +9789,7 @@ "version": "2.0.0", "resolved": "https://registry.npmjs.org/dom-serializer/-/dom-serializer-2.0.0.tgz", "integrity": "sha512-wIkAryiqt/nV5EQKqQpo3SToSOV9J0DnbJqwK7Wv/Trc92zIAYZ4FlMu+JPFW1DfGFt81ZTCGgDEabffXeLyJg==", + "dev": true, "license": "MIT", "dependencies": { "domelementtype": "^2.3.0", @@ -9803,6 +9804,7 @@ "version": "4.5.0", "resolved": "https://registry.npmjs.org/entities/-/entities-4.5.0.tgz", "integrity": "sha512-V0hjH4dGPh9Ao5p0MoRY6BVqtwCjhz6vI5LT8AJ55H+4g9/4vbHx1I54fS0XuclLhDHArPQCiMjDxjaL8fPxhw==", + "dev": true, "license": "BSD-2-Clause", "engines": { "node": ">=0.12" @@ -9815,6 +9817,7 @@ "version": "2.3.0", "resolved": "https://registry.npmjs.org/domelementtype/-/domelementtype-2.3.0.tgz", "integrity": "sha512-OLETBj6w0OsagBwdXnPdN0cnMfF9opN69co+7ZrbfPGrdpPVNBUj02spi6B1N7wChLQiPn4CSH/zJvXw56gmHw==", + "dev": true, "funding": [ { "type": "github", @@ -9827,6 +9830,7 @@ "version": "5.0.3", "resolved": "https://registry.npmjs.org/domhandler/-/domhandler-5.0.3.tgz", "integrity": "sha512-cgwlv/1iFQiFnU96XXgROh8xTeetsnJiDsTc7TYCLFd9+/WNkIqPTxiM/8pSd8VIrhXGTf1Ny1q1hquVqDJB5w==", + "dev": true, "license": "BSD-2-Clause", "dependencies": { "domelementtype": "^2.3.0" @@ -9842,6 +9846,7 @@ "version": "3.2.2", "resolved": "https://registry.npmjs.org/domutils/-/domutils-3.2.2.tgz", "integrity": "sha512-6kZKyUajlDuqlHKVX1w7gyslj9MPIXzIFiz/rGu35uC1wMi+kMhQwGhl4lt9unC9Vb9INnY9Z3/ZA3+FhASLaw==", + "dev": true, "license": "BSD-2-Clause", "dependencies": { "dom-serializer": "^2.0.0", @@ -9968,6 +9973,7 @@ "version": "7.0.1", "resolved": "https://registry.npmjs.org/entities/-/entities-7.0.1.tgz", "integrity": "sha512-TWrgLOFUQTH994YUyl1yT4uyavY5nNB5muff+RtWaqNVCAK408b5ZnnbNAUEWLTCpum9w6arT70i1XdQ4UeOPA==", + "dev": true, "license": "BSD-2-Clause", "engines": { "node": ">=0.12" @@ -11747,6 +11753,7 @@ "version": "10.1.0", "resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-10.1.0.tgz", "integrity": "sha512-VTZkM9GWRAtEpveh7MSF6SjjrpNVNNVJfFup7xTY3UpFtm67foy9HDVXneLtFVt4pMz5kZtgNcvCniNFb1hlEQ==", + "dev": true, "funding": [ "https://github.com/fb55/htmlparser2?sponsor=1", { @@ -18575,18 +18582,122 @@ "license": "MIT" }, "node_modules/sanitize-html": { - "version": "2.17.5", - "resolved": "https://registry.npmjs.org/sanitize-html/-/sanitize-html-2.17.5.tgz", - "integrity": "sha512-ZmU1joGRrvoyctKIiuwUxqR6moLoU2Wk+2bMccN6f7UwhAmwYDvWziqPxRDDN2Qip62NqnIrVrT9akbL6Wretg==", + "version": "2.17.7", + "resolved": "https://registry.npmjs.org/sanitize-html/-/sanitize-html-2.17.7.tgz", + "integrity": "sha512-PGtEkc9cbnedU3s9TmzDbpsZ8w086g/0Q8k8/oIO1NLNU3i5k9yn835CrjJSajp1KMmkisbO1qPXxNKO3welAg==", "license": "MIT", "dependencies": { "deepmerge": "^4.2.2", "escape-string-regexp": "^4.0.0", - "htmlparser2": "^10.1.0", + "htmlparser2": "^12.0.0", "is-plain-object": "^5.0.0", "launder": "^1.7.1", "parse-srcset": "^1.0.2", "postcss": "^8.3.11" + }, + "engines": { + "node": ">=22.12.0" + } + }, + "node_modules/sanitize-html/node_modules/dom-serializer": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/dom-serializer/-/dom-serializer-3.1.1.tgz", + "integrity": "sha512-4MEa38/QexBob6gFNwu+EGdWvhJ1OKuNwdYY3Y3NyeWDQfnGeDYQUDfIRzWu5B5gsv03so2Uxd28YC6zrsx3Lw==", + "license": "MIT", + "dependencies": { + "domelementtype": "^3.0.0", + "domhandler": "^6.0.0", + "entities": "^8.0.0" + }, + "engines": { + "node": ">=20.19.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/cheeriojs/dom-serializer?sponsor=1" + } + }, + "node_modules/sanitize-html/node_modules/domelementtype": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/domelementtype/-/domelementtype-3.0.0.tgz", + "integrity": "sha512-umCQid3jKbDmVjx8jGaW7uUykm4DEUeyV21hPxNMo2nV955DhUThwqyOIDtreepP31hl84X7G5U9ZfsWvIB3Pg==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/fb55" + } + ], + "license": "BSD-2-Clause", + "engines": { + "node": ">=20.19.0" + } + }, + "node_modules/sanitize-html/node_modules/domhandler": { + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/domhandler/-/domhandler-6.0.1.tgz", + "integrity": "sha512-gYzvtM72ZtxQO0T048kd6HWSbbGCNOUwcnfQ01cqIJ4X2IYKFFHZ5mKvrQETcFXxsRObZulDaKmy//R7TPtsBg==", + "license": "BSD-2-Clause", + "dependencies": { + "domelementtype": "^3.0.0" + }, + "engines": { + "node": ">=20.19.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/fb55/domhandler?sponsor=1" + } + }, + "node_modules/sanitize-html/node_modules/domutils": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/domutils/-/domutils-4.0.2.tgz", + "integrity": "sha512-qI4JLRKnSzqFqr7hAlS5xQDusBCjKSEG4t4+7aNrIQMHBcsC2TGEhuyABJdYkgSewL57PNLYEiibY2iPKhKpaA==", + "license": "BSD-2-Clause", + "dependencies": { + "dom-serializer": "^3.0.0", + "domelementtype": "^3.0.0", + "domhandler": "^6.0.0" + }, + "engines": { + "node": ">=20.19.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/fb55/domutils?sponsor=1" + } + }, + "node_modules/sanitize-html/node_modules/entities": { + "version": "8.1.0", + "resolved": "https://registry.npmjs.org/entities/-/entities-8.1.0.tgz", + "integrity": "sha512-kxL7msIffSuh9aaFAMD7rxAIuTRMAHMeBtgHW2yUdWw732ZNh4MehkF2gdjvtdmikkaIP9bFDDJOPlsvm7avrA==", + "license": "BSD-2-Clause", + "engines": { + "node": ">=20.19.0" + }, + "funding": { + "url": "https://github.com/fb55/entities?sponsor=1" + } + }, + "node_modules/sanitize-html/node_modules/htmlparser2": { + "version": "12.0.0", + "resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-12.0.0.tgz", + "integrity": "sha512-Tz7u1i95/g2x2jz81+x0FBVhBhY5aRTvD3tXXdFaljuNdzDLJ8UGNRrTcj2cgQvAg3iW/h77Fz15nLW0L0CrZw==", + "funding": [ + "https://github.com/fb55/htmlparser2?sponsor=1", + { + "type": "github", + "url": "https://github.com/sponsors/fb55" + } + ], + "license": "MIT", + "dependencies": { + "domelementtype": "^3.0.0", + "domhandler": "^6.0.0", + "domutils": "^4.0.2", + "entities": "^8.0.0" + }, + "engines": { + "node": ">=20.19.0" } }, "node_modules/sax": { diff --git a/package.json b/package.json index 58779d593..cec7e3739 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "react-dropzone": "^14.2.3", "react-hook-form": "7.54.2", "react-infinite-scroll-hook": "^4.1.1", - "sanitize-html": "^2.8.1", + "sanitize-html": "^2.17.7", "use-deep-compare-effect": "^1.8.1", "yup": "^0.32.11" }, From 061d53132fa8f24838a1aed6a7cef4fd8267b480 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:59:17 +0800 Subject: [PATCH 09/26] [MOL-22453][SX] Allow Jest to transform sanitize-html's ESM-only dependency chain sanitize-html 2.17.x depends on htmlparser2 v12+, which (along with dom-serializer/domhandler/domutils/domelementtype/entities) ships as ESM-only. The existing transformIgnorePatterns only carved out @lifesg/react-design-system and leaflet, so every test that imports sanitize-html (directly or via the shared Sanitize component) fails to even parse. Extend the exception list so babel-jest transpiles this dependency chain like project source. --- jest/jest.config.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/jest/jest.config.js b/jest/jest.config.js index 9ac1f6933..3a6e1d284 100644 --- a/jest/jest.config.js +++ b/jest/jest.config.js @@ -25,7 +25,11 @@ module.exports = async () => ({ ], verbose: true, bail: false, - transformIgnorePatterns: ["/node_modules/(?!@lifesg/react-design-system|leaflet)"], + // sanitize-html >=2.15 pulls in htmlparser2 v12+, which is ESM-only (and its own + // deps domhandler/domutils/domelementtype/entities), so it needs to go through babel too + transformIgnorePatterns: [ + "/node_modules/(?!@lifesg/react-design-system|leaflet|sanitize-html|htmlparser2|dom-serializer|domhandler|domutils|domelementtype|entities)", + ], transform: { "\\.[jt]sx?$": ["babel-jest", { excludeJestPreset: true }], "^.+\\.css$": "jest-transform-css", From 674abdd838b2b9943e336113b3b50f18b7a9f14b Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:59:30 +0800 Subject: [PATCH 10/26] [MOL-22453][SX][asgard-0004] Harden local Playwright server container exposure network_mode: "host" plus binding the Playwright automation server to 0.0.0.0 exposed it on every interface of the host's own network namespace, unauthenticated, to anything else on that machine or LAN (host networking bypasses Docker's usual port isolation entirely). Drop network_mode: "host" so the container gets its own isolated network namespace, and publish the server's port only on the host's loopback interface via `ports: ["127.0.0.1:3010:3010"]`. The server itself still binds 0.0.0.0 inside the container - that's required for Docker's own NAT to forward anything to it at all; the actual exposure boundary is the host-side publish, not the in-container bind. (An earlier version of this fix tried binding 127.0.0.1 inside the container instead, which seemed like it would be even more locked-down, but actually running the e2e pipeline showed it made the server completely unreachable even from the host - loopback traffic never leaves a container's own network namespace, regardless of Docker networking mode or port publishing. `docker port` and a `curl` from the host confirmed the loopback-only publish here preserves connectivity while keeping the LAN/host-namespace exposure closed.) playwright.config.ts only ever connects via ws://127.0.0.1:3010/, which this repo's actual CI scripts (e2e-ci.sh/e2e-setup.sh) run through directly - not a theoretical path, verified end-to-end. --- docker-compose.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index f72cf6aa3..d87bd2eb3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,6 +1,7 @@ services: playwright-tests: build: . - network_mode: "host" + ports: + - "127.0.0.1:3010:3010" extra_hosts: - "host.docker.internal:host-gateway" From 04d5f73adff2f904aebe7599ab051d539945644f Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 16:59:54 +0800 Subject: [PATCH 11/26] [MOL-22453][SX][asgard-0013] Restrict FileUpload prefill fileUrl to an origin allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prefilling a file-upload field via a schema/API-supplied fileUrl made the browser fetch that URL directly, carrying the user's ambient session credentials (cookies, auth headers via AxiosApiClient). A schema or API response that can influence fileUrl (e.g. a compromised backend response, or a misconfigured prefill source) could point it at an internal/third-party endpoint and exfiltrate the authenticated response — a client-side SSRF/credential-leak vector. Add a required allowedFileOrigins schema field. fileUrl is only fetched if its origin is in the allowlist; if allowedFileOrigins is unset, the fetch is skipped entirely, same as if no fileUrl were provided at all. Fail closed, not open: there's no principled permissive default here — this codebase has no basis for guessing which origins are safe for a given consumer's file storage backend, so "don't fetch until told it's safe" is the only default that's actually safe. This is a real breaking change for any consumer currently relying on unconfigured fileUrl prefetching. A downstream check across the 18 in-scope repos (see MOL-22453-impact-report.md) found none currently depend on this via the library's FileUpload field — one repo (facilities-admin-web) has a superficially similar fileUrl-fetch pattern, but it's built on the raw @lifesg/react-design-system FileUpload component directly with its own custom fetch logic, not this library's schema-driven field, so it's unaffected. The 9 out-of-scope BSG/RBS repos were not checked and should be verified by their own teams before upgrading. --- .../fields/file-upload/file-upload.spec.tsx | 43 ++++++++++++++++++- .../fields/file-upload/file-upload-manager.ts | 21 ++++++++- .../fields/file-upload/file-upload.tsx | 2 + src/components/fields/file-upload/types.ts | 5 +++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/__tests__/components/fields/file-upload/file-upload.spec.tsx b/src/__tests__/components/fields/file-upload/file-upload.spec.tsx index ad9de21c5..fcedbfe6c 100644 --- a/src/__tests__/components/fields/file-upload/file-upload.spec.tsx +++ b/src/__tests__/components/fields/file-upload/file-upload.spec.tsx @@ -201,10 +201,31 @@ describe(UI_TYPE, () => { ); }); - it("should support default value based on fileUrl", async () => { + it("should not fetch a prefilled fileUrl when allowedFileOrigins is not set", async () => { + const getSpy = jest.spyOn(AxiosApiClient.prototype, "get").mockResolvedValue(FILE_1); + const fileUrl = "https://example.com/path/to/file"; + await renderComponent({ + overrideSchema: { + defaultValues: { + [COMPONENT_ID]: [{ fileUrl, fileId: FILE_1.name, fileName: FILE_1.name }], + }, + }, + }); + await act(async () => { + await flushPromise(200); + }); + + expect(getSpy).not.toHaveBeenCalled(); + await waitFor(() => { + expect(screen.getByText("0 KB")).toBeInTheDocument(); + }); + }); + + it("should fetch fileUrl when its origin is in allowedFileOrigins", async () => { jest.spyOn(AxiosApiClient.prototype, "get").mockResolvedValue(FILE_1); - const fileUrl = "dummy url"; + const fileUrl = "https://trusted.example.com/path/to/file"; await renderComponent({ + overrideField: { allowedFileOrigins: ["https://trusted.example.com"] }, overrideSchema: { defaultValues: { [COMPONENT_ID]: [{ fileUrl, fileId: FILE_1.name, fileName: FILE_1.name }], @@ -231,6 +252,24 @@ describe(UI_TYPE, () => { ); }); + it("should not fetch fileUrl when its origin is not in allowedFileOrigins", async () => { + const getSpy = jest.spyOn(AxiosApiClient.prototype, "get").mockResolvedValue(FILE_1); + const fileUrl = "https://untrusted.example.com/path/to/file"; + await renderComponent({ + overrideField: { allowedFileOrigins: ["https://trusted.example.com"] }, + overrideSchema: { + defaultValues: { + [COMPONENT_ID]: [{ fileUrl, fileId: FILE_1.name, fileName: FILE_1.name }], + }, + }, + }); + await act(async () => { + await flushPromise(200); + }); + + expect(getSpy).not.toHaveBeenCalled(); + }); + it("should support default value without dataURL and fileUrl", async () => { jest.spyOn(AxiosApiClient.prototype, "get").mockResolvedValue(FILE_1); await renderComponent({ diff --git a/src/components/fields/file-upload/file-upload-manager.ts b/src/components/fields/file-upload/file-upload-manager.ts index e1d7babfb..3fdda43a5 100644 --- a/src/components/fields/file-upload/file-upload-manager.ts +++ b/src/components/fields/file-upload/file-upload-manager.ts @@ -15,6 +15,7 @@ import { } from "./types"; interface IProps { + allowedFileOrigins?: string[] | undefined; compressImages: boolean; fileTypeRule: IFileUploadValidationRule; fileExtensionRule: IFileUploadValidationRule; @@ -28,11 +29,29 @@ interface IProps { const RESIZEABLE_IMAGE_TYPES = ["image/jpeg", "image/gif", "image/png"]; +const isAllowedFileOrigin = (url: string, allowedFileOrigins: string[] | undefined): boolean => { + if (!allowedFileOrigins?.length) { + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.warn( + `FileUpload: rejecting fileUrl "${url}" — allowedFileOrigins must be set on the schema for a fileUrl to be fetched.` + ); + } + return false; + } + try { + return allowedFileOrigins.includes(new URL(url).origin); + } catch { + return false; + } +}; + const FileUploadManager = (props: IProps) => { // ============================================================================= // CONST, STATE, REFS // ============================================================================= const { + allowedFileOrigins, compressImages, fileTypeRule, fileExtensionRule, @@ -284,7 +303,7 @@ const FileUploadManager = (props: IProps) => { if (fileToInject.dataURL) { const blob = await FileHelper.dataUrlToBlob(fileToInject.dataURL); rawFile = new File([blob], fileToInject.rawFile.name); - } else if (fileToInject.fileUrl) { + } else if (fileToInject.fileUrl && isAllowedFileOrigin(fileToInject.fileUrl, allowedFileOrigins)) { const response: Blob = await new AxiosApiClient("", undefined, undefined, false, { responseType: "blob", }).get(fileToInject.fileUrl); diff --git a/src/components/fields/file-upload/file-upload.tsx b/src/components/fields/file-upload/file-upload.tsx index 45524eb33..3bc3f63c0 100644 --- a/src/components/fields/file-upload/file-upload.tsx +++ b/src/components/fields/file-upload/file-upload.tsx @@ -33,6 +33,7 @@ export const FileUploadInner = (props: IGenericFieldProps) => isTouched, value, schema: { + allowedFileOrigins, compressImages, description, hideThumbnail, @@ -312,6 +313,7 @@ export const FileUploadInner = (props: IGenericFieldProps) => <> }; warning?: string | undefined; compressImages?: boolean | undefined; + /** origins allowed when fetching a prefilled `fileUrl`. Must be set for a prefilled fileUrl to be fetched at + * all — when unset, the fetch is skipped and the file falls through to the same unverified-file handling as + * a missing fileUrl. There is no permissive default: this codebase has no basis for guessing which origins + * are safe for a given consumer's file storage backend. */ + allowedFileOrigins?: string[] | undefined; } export enum EFileStatus { From 9bb217007ba2c9a998c12c8e66dcdd427cf93a48 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:00:08 +0800 Subject: [PATCH 12/26] [MOL-22453][SX][asgard-0014] Stop trusting caller-supplied file metadata by default When a prefilled file had neither a dataURL nor a fetchable fileUrl, injectFile fell back to validating (and reporting the size of) the file purely from the caller-supplied uploadResponse object - mimeType/ext/fileSize the browser never independently verified. A compromised or misconfigured prefill source could claim any type/size for a file, bypassing fileType/fileExtension/maxSizeInKb validation entirely for that upload. Add an optional trustProvidedFileMetadata schema flag. Left unset (new default), such files are flagged as unverified and rejected with a generic upload error instead of being silently validated against unverified metadata. Set to true to restore the previous behavior for schemas/backends that intentionally rely on it. This is a deliberate breaking change to the default behavior. Checked both local clones and the wider GitLab org for consumers relying on the old default (uploadResponse-only prefill, no fileUrl/dataURL) and found none. Should still be called out explicitly in release notes. Running the real e2e suite against this change caught exactly the kind of consumer it would affect: the file-upload "Thumbnail" demo prefills a file via uploadResponse only, with no dataURL/fileUrl, and rendered as a rejected/unverified card instead of the expected thumbnail once this default flipped. Opted that demo into trustProvidedFileMetadata: true, since showing a trusted-metadata thumbnail is what it's actually demonstrating - verified the "Thumbnail" e2e test passes again after this change. --- .../fields/file-upload/thumbnail.e2e.tsx | 4 +++ .../fields/file-upload/file-upload.spec.tsx | 33 +++++++++++++++++++ .../fields/file-upload/file-upload-manager.ts | 16 ++++++--- .../fields/file-upload/file-upload.tsx | 2 ++ src/components/fields/file-upload/types.ts | 4 +++ 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/e2e/nextjs-app/app/components/fields/file-upload/thumbnail.e2e.tsx b/e2e/nextjs-app/app/components/fields/file-upload/thumbnail.e2e.tsx index 15a1daa08..9a1fed1f6 100644 --- a/e2e/nextjs-app/app/components/fields/file-upload/thumbnail.e2e.tsx +++ b/e2e/nextjs-app/app/components/fields/file-upload/thumbnail.e2e.tsx @@ -15,6 +15,10 @@ const SCHEMA: IFrontendEngineData = { type: "base64", url: "/api/upload", }, + // this demo prefills a file via uploadResponse only (no dataURL/fileUrl); since + // asgard-0014, that metadata is untrusted by default and the file is rejected as + // unverified unless the schema explicitly opts in + trustProvidedFileMetadata: true, }, }, }, diff --git a/src/__tests__/components/fields/file-upload/file-upload.spec.tsx b/src/__tests__/components/fields/file-upload/file-upload.spec.tsx index fcedbfe6c..cb8913d2a 100644 --- a/src/__tests__/components/fields/file-upload/file-upload.spec.tsx +++ b/src/__tests__/components/fields/file-upload/file-upload.spec.tsx @@ -268,6 +268,9 @@ describe(UI_TYPE, () => { }); expect(getSpy).not.toHaveBeenCalled(); + await waitFor(() => { + expect(screen.getAllByText(ERROR_MESSAGES.UPLOAD().GENERIC).length).toBe(2); + }); }); it("should support default value without dataURL and fileUrl", async () => { @@ -307,6 +310,7 @@ describe(UI_TYPE, () => { uploadedAt: "2025-01-01T03:57:55.573Z", }; await renderComponent({ + overrideField: { trustProvidedFileMetadata: true }, overrideSchema: { defaultValues: { [COMPONENT_ID]: [{ fileId: FILE_1.name, fileName: FILE_1.name, uploadResponse }], @@ -334,6 +338,7 @@ describe(UI_TYPE, () => { }, }; await renderComponent({ + overrideField: { trustProvidedFileMetadata: true }, overrideSchema: { defaultValues: { [COMPONENT_ID]: [{ fileId: FILE_1.name, fileName: FILE_1.name, uploadResponse }], @@ -350,6 +355,30 @@ describe(UI_TYPE, () => { }); }); + it("should reject prefilled file with no dataURL/fileUrl by default (trustProvidedFileMetadata unset)", async () => { + const uploadResponse = { + fileId: "f307b120-6c4d-4b2c-b278-33bb9aefbc6e", + fileName: "my-image.jpg", + fileSize: 595705, + mimeType: "image/jpeg", + }; + await renderComponent({ + overrideSchema: { + defaultValues: { + [COMPONENT_ID]: [{ fileId: FILE_1.name, fileName: FILE_1.name, uploadResponse }], + }, + }, + }); + await act(async () => { + await flushPromise(200); + }); + + expect(uploadSpy).not.toHaveBeenCalled(); + await waitFor(() => { + expect(screen.getAllByText(ERROR_MESSAGES.UPLOAD().GENERIC).length).toBe(2); + }); + }); + it("should display 0kb when dataURL and fileUrl are not present", async () => { await renderComponent({ overrideSchema: { @@ -377,6 +406,7 @@ describe(UI_TYPE, () => { uploadedAt: "2025-01-01T03:57:55.573Z", }; await renderComponent({ + overrideField: { trustProvidedFileMetadata: true }, overrideSchema: { defaultValues: { [COMPONENT_ID]: [{ fileId: FILE_1.name, fileName: FILE_1.name, uploadResponse }], @@ -404,6 +434,7 @@ describe(UI_TYPE, () => { }, }; await renderComponent({ + overrideField: { trustProvidedFileMetadata: true }, overrideSchema: { defaultValues: { [COMPONENT_ID]: [{ fileId: FILE_1.name, fileName: FILE_1.name, uploadResponse }], @@ -634,6 +665,7 @@ describe(UI_TYPE, () => { async ({ type, validation, defaultValues }) => { await renderComponent({ overrideField: { + trustProvidedFileMetadata: true, uploadOnAddingFile: { type, url: UPLOAD_URL }, validation: [{ ...validation, errorMessage: ERROR_MESSAGE }], }, @@ -685,6 +717,7 @@ describe(UI_TYPE, () => { async ({ type, validation, defaultValues }) => { await renderComponent({ overrideField: { + trustProvidedFileMetadata: true, uploadOnAddingFile: { type, url: UPLOAD_URL }, validation: [{ ...validation, errorMessage: ERROR_MESSAGE }], }, diff --git a/src/components/fields/file-upload/file-upload-manager.ts b/src/components/fields/file-upload/file-upload-manager.ts index 3fdda43a5..677de331d 100644 --- a/src/components/fields/file-upload/file-upload-manager.ts +++ b/src/components/fields/file-upload/file-upload-manager.ts @@ -22,6 +22,7 @@ interface IProps { hideThumbnail?: boolean | undefined; id: string; maxFileSizeRule: IFileUploadValidationRule; + trustProvidedFileMetadata: boolean; upload: IFileUploadSchema["uploadOnAddingFile"]; uploadRule: IFileUploadValidationRule; value: IFileUploadValue[]; @@ -58,6 +59,7 @@ const FileUploadManager = (props: IProps) => { hideThumbnail, id, maxFileSizeRule, + trustProvidedFileMetadata, upload, uploadRule, value, @@ -313,14 +315,20 @@ const FileUploadManager = (props: IProps) => { } // rawFile may not be available because some use cases is not able to return dataURL / fileUrl due to security concerns - // in such cases, we will rely on the uploadResponse for file info - const uploadData = fileToInject.uploadResponse?.["data"] || fileToInject.uploadResponse; + // in such cases, uploadResponse is only trusted for file info when the schema explicitly opts in via + // trustProvidedFileMetadata — that metadata is caller-supplied and not independently verified otherwise + const canTrustUploadResponse = !rawFile && trustProvidedFileMetadata; + const uploadData = canTrustUploadResponse + ? fileToInject.uploadResponse?.["data"] || fileToInject.uploadResponse + : undefined; const { errorMessage, fileType } = rawFile ? await readFile({ ...fileToInject, rawFile }) - : validateFileType({ + : canTrustUploadResponse + ? validateFileType({ mime: uploadData?.["mimeType"], ext: uploadData?.["ext"], - }); + }) + : { errorMessage: ERROR_MESSAGES.UPLOAD().GENERIC, fileType: undefined }; let size = rawFile?.size || uploadData?.["fileSize"] || 0; if (isNaN(size)) { diff --git a/src/components/fields/file-upload/file-upload.tsx b/src/components/fields/file-upload/file-upload.tsx index 3bc3f63c0..15c00285c 100644 --- a/src/components/fields/file-upload/file-upload.tsx +++ b/src/components/fields/file-upload/file-upload.tsx @@ -38,6 +38,7 @@ export const FileUploadInner = (props: IGenericFieldProps) => description, hideThumbnail, label, + trustProvidedFileMetadata, uploadOnAddingFile, validation, warning: schemaWarning, @@ -320,6 +321,7 @@ export const FileUploadInner = (props: IGenericFieldProps) => hideThumbnail={hideThumbnail} id={id} maxFileSizeRule={maxFileSizeRuleRef.current} + trustProvidedFileMetadata={!!trustProvidedFileMetadata} upload={uploadOnAddingFile} uploadRule={uploadRuleRef.current} value={value} diff --git a/src/components/fields/file-upload/types.ts b/src/components/fields/file-upload/types.ts index cb5a54325..ae348077f 100644 --- a/src/components/fields/file-upload/types.ts +++ b/src/components/fields/file-upload/types.ts @@ -38,6 +38,10 @@ export interface IFileUploadSchema * a missing fileUrl. There is no permissive default: this codebase has no basis for guessing which origins * are safe for a given consumer's file storage backend. */ allowedFileOrigins?: string[] | undefined; + /** when true, prefilled files without a fetchable dataURL/fileUrl are validated against the caller-supplied + * `uploadResponse` metadata (mimeType/ext/fileSize) as-is. Defaults to false, which treats such files as + * unverified since that metadata is not independently checked */ + trustProvidedFileMetadata?: boolean | undefined; } export enum EFileStatus { From ef7255f2c360a9060f336a9b8029ce886c6baf3c Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:00:22 +0800 Subject: [PATCH 13/26] [MOL-22453][SX][asgard-0015] Document that OTP verified state is client-asserted, not a security boundary No code change. IOtpVerificationValue.state/additionalData are set by this component once its own client-side verify-OTP API call succeeds, but the submitted form payload is just JSON a client fully controls. A consuming backend that trusts state === "verified" at face value without independently re-verifying the OTP transaction server-side is trivially bypassable. Documents the existing trust boundary so integrators don't assume the frontend enforces it. --- src/components/fields/otp-verification-field/types.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/components/fields/otp-verification-field/types.ts b/src/components/fields/otp-verification-field/types.ts index b585486f4..70a2e2414 100644 --- a/src/components/fields/otp-verification-field/types.ts +++ b/src/components/fields/otp-verification-field/types.ts @@ -51,6 +51,13 @@ export interface IOtpVerificationValue { contact: string; prefix?: string | undefined; type: TOtpVerificationType; + /** + * client-asserted, not a security boundary: the frontend sets this to "verified" once its own OTP + * verification call succeeds, but the submitted form payload is just JSON a client controls. A + * consuming backend must independently re-verify the OTP transaction (e.g. by its transaction ID) + * before trusting a submission with state "verified" — never trust this field alone server-side. + */ state: "sent" | "verified" | "default"; + /** client-asserted, same caveat as `state` above — not independently verified by this component */ additionalData?: unknown; } From b2ce51fbdd87ed0c6f6980c504e71a546e1dbf62 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:00:38 +0800 Subject: [PATCH 14/26] [MOL-22453][SX][asgard-0009] Bound input length before testing schema-authored regex in Yup "matches" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "matches" validation rule compiled a schema-authored /pattern/ string and passed it straight to Yup's .matches(), which runs regex.search() against arbitrary-length form field input with no bound. A catastrophic-backtracking pattern (an easy, common authoring mistake, e.g. /^(a+)+$/) hangs the tab once a user types input that triggers it — no schema-author compromise needed, just a vulnerable pattern plus ordinary form input. Introduce a shared RegexHelper (parseMatchesPattern/safeTestRegex) that skips the regex test entirely once input exceeds a safe length bound (500 chars), and switch "matches" to a custom .test() built on it instead of .matches() (which has no hook for this). This reduces worst-case backtracking cost for the common cases (form field values, filenames, short keystroke buffers) but does not make a genuinely pathological pattern safe against every input length — a stronger fix (linting schema-authored patterns for catastrophic-backtracking shapes) is a further-out improvement, not attempted here. The same utility is applied to three sibling sinks using the same /pattern/ flags parsing convention in the following commits. --- .../frontend-engine/yup/yup-helper.spec.ts | 9 ++++ src/__tests__/utils/regex-helper.spec.ts | 43 +++++++++++++++++++ src/context-providers/yup/helper.ts | 17 +++++--- src/utils/index.ts | 1 + src/utils/regex-helper.ts | 28 ++++++++++++ 5 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 src/__tests__/utils/regex-helper.spec.ts create mode 100644 src/utils/regex-helper.ts diff --git a/src/__tests__/components/frontend-engine/yup/yup-helper.spec.ts b/src/__tests__/components/frontend-engine/yup/yup-helper.spec.ts index 2f4b4a75d..c4e8f2780 100644 --- a/src/__tests__/components/frontend-engine/yup/yup-helper.spec.ts +++ b/src/__tests__/components/frontend-engine/yup/yup-helper.spec.ts @@ -248,6 +248,15 @@ describe("YupHelper", () => { ); }); + it("should not hang on a pathological matches pattern when input exceeds the safe length bound", () => { + const schema = YupHelper.mapRules(Yup.string(), [{ matches: "/^(a+)+$/", errorMessage: ERROR_MESSAGE }]); + const maliciousValue = `${"a".repeat(1000)}!`; + + const start = Date.now(); + expect(TestHelper.getError(() => schema.validateSync(maliciousValue))?.message).toBe(ERROR_MESSAGE); + expect(Date.now() - start).toBeLessThan(1000); + }); + const generateMultipleFieldSchema = (type: "string" | "number" | "boolean" | "object" | "array") => YupHelper.buildSchema({ field1: { schema: YupHelper.mapSchemaType(type), validationRules: [] }, diff --git a/src/__tests__/utils/regex-helper.spec.ts b/src/__tests__/utils/regex-helper.spec.ts new file mode 100644 index 000000000..b23091cff --- /dev/null +++ b/src/__tests__/utils/regex-helper.spec.ts @@ -0,0 +1,43 @@ +import { RegexHelper } from "../../utils"; + +describe("regex-helper", () => { + describe("parseMatchesPattern", () => { + it("should parse a /pattern/flags string into a RegExp", () => { + const regex = RegexHelper.parseMatchesPattern("/^hello/i"); + + expect(regex).toBeInstanceOf(RegExp); + expect(regex.source).toBe("^hello"); + expect(regex.flags).toBe("i"); + }); + + it("should fall back to treating the whole string as a pattern when it has no /pattern/flags wrapper", () => { + const regex = RegexHelper.parseMatchesPattern("hello"); + + expect(regex).toBeInstanceOf(RegExp); + expect(regex.source).toBe("hello"); + }); + + it("should return undefined instead of throwing on an invalid pattern", () => { + expect(RegexHelper.parseMatchesPattern("/[/")).toBeUndefined(); + }); + }); + + describe("safeTestRegex", () => { + it("should return false when regex is undefined", () => { + expect(RegexHelper.safeTestRegex(undefined, "hello")).toBe(false); + }); + + it("should test the value against the regex when within the safe length bound", () => { + expect(RegexHelper.safeTestRegex(/^hello/, "hello world")).toBe(true); + expect(RegexHelper.safeTestRegex(/^hello/, "goodbye world")).toBe(false); + }); + + it("should not hang and should return false when value exceeds the safe length bound, even for a pathological pattern", () => { + const maliciousValue = `${"a".repeat(RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH + 1)}!`; + + const start = Date.now(); + expect(RegexHelper.safeTestRegex(/^(a+)+$/, maliciousValue)).toBe(false); + expect(Date.now() - start).toBeLessThan(1000); + }); + }); +}); diff --git a/src/context-providers/yup/helper.ts b/src/context-providers/yup/helper.ts index 90f147efd..9a3612bd9 100644 --- a/src/context-providers/yup/helper.ts +++ b/src/context-providers/yup/helper.ts @@ -1,6 +1,7 @@ import * as Yup from "yup"; import { ObjectShape } from "yup/lib/object"; import { ERROR_MESSAGES } from "../../components/shared"; +import { RegexHelper } from "../../utils"; import { IFieldYupConfig, IYupConditionalValidationRule, @@ -187,13 +188,15 @@ export namespace YupHelper { break; case !!rule.matches: { - const matches = rule.matches.match(/\/(.*)\/([a-z]+)?/); - try { - yupSchema = (yupSchema as Yup.StringSchema).matches( - new RegExp(matches[1], matches[2]), - rule.errorMessage - ); - } catch (error) { + const regex = RegexHelper.parseMatchesPattern(rule.matches); + if (regex) { + yupSchema = (yupSchema as Yup.StringSchema).test({ + name: "matches", + message: rule.errorMessage, + test: (value) => + value === undefined || value === null || RegexHelper.safeTestRegex(regex, value), + }); + } else { console.warn(`error applying "${ruleKey}" condition to ${yupSchema.type} schema`); } } diff --git a/src/utils/index.ts b/src/utils/index.ts index 89bd1b231..d56b3cc2f 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -9,3 +9,4 @@ export * from "./object-helper"; export * from "./test-helper"; export * from "./types"; export * from "./prop-helper"; +export * from "./regex-helper"; diff --git a/src/utils/regex-helper.ts b/src/utils/regex-helper.ts new file mode 100644 index 000000000..07e17d1ad --- /dev/null +++ b/src/utils/regex-helper.ts @@ -0,0 +1,28 @@ +export namespace RegexHelper { + /** upper bound on the string length tested against a schema-authored regex pattern. + * A genuinely pathological pattern can still backtrack catastrophically below this length, + * but bounding it caps the worst-case cost for the common cases (filenames, form field values, + * short keystroke buffers) this is applied against. */ + export const MAX_SAFE_PATTERN_INPUT_LENGTH = 500; + + /** parses a `/pattern/flags`-style string into a RegExp, matching the convention already used + * across the codebase for schema-authored regex config. Returns undefined instead of throwing + * on an invalid pattern. */ + export const parseMatchesPattern = (pattern: string): RegExp | undefined => { + try { + const parsed = pattern.match(/^\/(.+)\/([a-z]*)$/i); + return parsed ? new RegExp(parsed[1], parsed[2]) : new RegExp(pattern); + } catch { + return undefined; + } + }; + + /** tests `value` against `regex`, bailing out instead of running .test() when `value` exceeds + * MAX_SAFE_PATTERN_INPUT_LENGTH. Reduces (but does not eliminate) worst-case ReDoS exposure for + * schema-authored patterns tested against attacker-influenceable strings. */ + export const safeTestRegex = (regex: RegExp | undefined, value: string): boolean => { + if (!regex) return false; + if (value.length > MAX_SAFE_PATTERN_INPUT_LENGTH) return false; + return regex.test(value); + }; +} From 7e0100ec2c931720bd972c1797cb4c46157be460 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:00:54 +0800 Subject: [PATCH 15/26] [MOL-22453][SX] Fix notMatches condition: crash on malformed regex + ReDoS bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit notMatches parsed its /pattern/flags string with no try/catch at all — a malformed regex string threw a raw, unhandled TypeError from matches[1] (regex.match() returns null on no match), crashing validation on bad input, not even malicious input. It also had no protection against a catastrophic-backtracking pattern being tested against arbitrary-length form field values, the same ReDoS class fixed for the "matches" rule in the previous commit (this is a second, previously-uncited sink for that same finding — the original report only pointed at yup/helper.ts's mapRules). Reuse RegexHelper from the previous commit: parse via parseMatchesPattern (fixes the crash), and reject an overly long value outright instead of testing it (fail-closed, matching this condition's inverted "does NOT match" semantics — unlike "matches", where skipping the test defaults to invalid, here it must default to invalid via an explicit length check rather than reusing safeTestRegex's return value directly). --- .../yup/custom-conditions.spec.ts | 22 +++++++++++++++++++ .../yup/custom-conditions/index.ts | 14 +++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/__tests__/components/frontend-engine/yup/custom-conditions.spec.ts b/src/__tests__/components/frontend-engine/yup/custom-conditions.spec.ts index 0b7b940ea..67195cba1 100644 --- a/src/__tests__/components/frontend-engine/yup/custom-conditions.spec.ts +++ b/src/__tests__/components/frontend-engine/yup/custom-conditions.spec.ts @@ -17,3 +17,25 @@ it.each` expect(TestHelper.getError(() => schema.validateSync(invalidValue)).message).toBe(ERROR_MESSAGE) ); }); + +describe("notMatches", () => { + const buildSchema = (regex: string) => + YupHelper.buildFieldSchema(YupHelper.mapSchemaType("string"), [ + { notMatches: regex, errorMessage: ERROR_MESSAGE }, + ]); + + it("should not throw when the regex string is malformed", () => { + const schema = buildSchema("not a /pattern/flags string ["); + + expect(() => schema.validateSync("hello")).not.toThrow(); + }); + + it("should reject an overly long value instead of testing it against the pattern", () => { + const schema = buildSchema("/^(a+)+$/"); + const maliciousValue = `${"a".repeat(600)}!`; + + const start = Date.now(); + expect(TestHelper.getError(() => schema.validateSync(maliciousValue)).message).toBe(ERROR_MESSAGE); + expect(Date.now() - start).toBeLessThan(1000); + }); +}); diff --git a/src/context-providers/yup/custom-conditions/index.ts b/src/context-providers/yup/custom-conditions/index.ts index 056271e78..dc71e4109 100644 --- a/src/context-providers/yup/custom-conditions/index.ts +++ b/src/context-providers/yup/custom-conditions/index.ts @@ -7,7 +7,7 @@ import { YupHelper } from "../helper"; import "./html-safe"; import "./uinfin"; import "./uen"; -import { DateTimeHelper } from "../../../utils"; +import { DateTimeHelper, RegexHelper } from "../../../utils"; import { IDaysRangeRule, IWhitespaceRule } from "../types"; /** @@ -23,8 +23,16 @@ YupHelper.addCondition("string", "notMatches", (value: string, regex: string) => if (isEmptyValue(value)) { return true; } - const matches = regex.match(/\/(.*)\/([a-z]+)?/); - const parsedRegex = new RegExp(matches[1], matches[2]); + const parsedRegex = RegexHelper.parseMatchesPattern(regex); + if (!parsedRegex) { + console.warn(`invalid regex pattern: ${regex}`); + return true; + } + if (value.length > RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH) { + // cannot safely test an overly long value against the pattern; conservatively treat it as + // violating the notMatches rule rather than silently letting it through unchecked + return false; + } return !parsedRegex.test(value); }); /** @deprecated */ From b0e82a6368b5d0e8c755591f23e43dc759753614 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:01:10 +0800 Subject: [PATCH 16/26] [MOL-22453][SX] Bound input length before testing filenameMatches pattern in ImageManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveMatchesPattern's try/catch only guards against a syntactically invalid regex; a syntactically valid catastrophic-backtracking pattern (e.g. /^(.+)+\.(jpg|png)$/ — an easy, common authoring mistake) compiled fine and hung later when tested against the uploaded file's name — attacker-controlled with no schema-author compromise needed, since a user picks their own filename on upload. Reuse RegexHelper from the earlier commits: parseMatchesPattern for parsing (same behavior, now shared) and safeTestRegex for the actual test, which skips testing an overly long filename and treats that as not matching (fail-closed, consistent with this being a "must match" pattern check). --- .../fields/image-upload/image-upload.spec.tsx | 15 +++++++++++++++ .../image-upload/image-manager/image-manager.ts | 13 +++---------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/__tests__/components/fields/image-upload/image-upload.spec.tsx b/src/__tests__/components/fields/image-upload/image-upload.spec.tsx index c4f58197e..b6109ef6c 100644 --- a/src/__tests__/components/fields/image-upload/image-upload.spec.tsx +++ b/src/__tests__/components/fields/image-upload/image-upload.spec.tsx @@ -380,6 +380,21 @@ describe("image-upload", () => { }) ); }); + + it("should not hang on a pathological pattern with a long filename", async () => { + const maliciousFile = new File(["file"], `${"a".repeat(600)}!.jpg`, { type: "image/jpeg" }); + + const start = Date.now(); + await renderComponent({ + files: [maliciousFile], + overrideField: { validation: [{ matches: "/^(a+)+$/", errorMessage: ERROR_MESSAGE }] }, + uploadType: "input", + }); + await waitFor(() => expect(screen.getByText(ERROR_MESSAGE)).toBeInTheDocument()); + + expect(Date.now() - start).toBeLessThan(1000); + expect(uploadSpy).not.toHaveBeenCalled(); + }); }); }); diff --git a/src/components/fields/image-upload/image-manager/image-manager.ts b/src/components/fields/image-upload/image-manager/image-manager.ts index ce0e13b96..52c7e350b 100644 --- a/src/components/fields/image-upload/image-manager/image-manager.ts +++ b/src/components/fields/image-upload/image-manager/image-manager.ts @@ -1,6 +1,6 @@ import { useContext, useEffect, useRef } from "react"; import { useFormContext } from "react-hook-form"; -import { AxiosApiClient, FileHelper, ImageHelper, generateRandomId } from "../../../../utils"; +import { AxiosApiClient, FileHelper, ImageHelper, RegexHelper, generateRandomId } from "../../../../utils"; import { useFieldEvent, usePrevious } from "../../../../utils/hooks"; import { ImageContext } from "../image-context"; import { @@ -110,7 +110,7 @@ export const ImageManager = (props: IProps) => { case EImageStatus.NONE: if (filenameMatches) { const pattern = resolveMatchesPattern(filenameMatches); - if (pattern && !pattern.test(image.name)) { + if (pattern && !RegexHelper.safeTestRegex(pattern, image.name)) { setImages((prev) => { const updatedImages = [...prev]; updatedImages[index] = { @@ -257,14 +257,7 @@ export const ImageManager = (props: IProps) => { * Converts a matches string (e.g. "/^abc$/i" or "^abc$") to a RegExp. * Returns undefined if the string is invalid. */ - const resolveMatchesPattern = (matches: string): RegExp | undefined => { - try { - const parsed = matches.match(/^\/(.+)\/([gimsuy]*)$/); - return parsed ? new RegExp(parsed[1], parsed[2] || "") : new RegExp(matches); - } catch { - return undefined; - } - }; + const resolveMatchesPattern = (matches: string): RegExp | undefined => RegexHelper.parseMatchesPattern(matches); const convertImage = async (index: number, image: IImage) => { try { From db09394607fe3fb40339b1a09fb700f66e011a49 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:01:26 +0800 Subject: [PATCH 17/26] [MOL-22453][SX] Use shared RegexHelper for maskRegex parsing in MaskedField MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getRegex duplicated the same ad hoc "/pattern/flags" parsing already fixed via RegexHelper in the previous three commits. Switch to parseMatchesPattern for consistency (same crash-safety as before, no behavior change). Note on scope: unlike the other three sinks, the actual regex .test() against user keystrokes happens inside the design-system MaskedInput component this field hands maskRegex off to, not in this file — this codebase doesn't own that test call, so the safeTestRegex length bound from the earlier commits can't be applied here. A catastrophic-backtracking maskRegex pattern tested against live keystrokes remains a real, unresolved exposure; fixing it would require a change in @lifesg/react-design-system's MaskedInput itself. --- .../components/fields/masked-field/masked-field.spec.tsx | 9 +++++++++ src/components/fields/masked-field/masked-field.tsx | 9 ++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/__tests__/components/fields/masked-field/masked-field.spec.tsx b/src/__tests__/components/fields/masked-field/masked-field.spec.tsx index 573effd8f..ddfec6fce 100644 --- a/src/__tests__/components/fields/masked-field/masked-field.spec.tsx +++ b/src/__tests__/components/fields/masked-field/masked-field.spec.tsx @@ -99,6 +99,15 @@ describe(UI_TYPE, () => { expect(SUBMIT_FN).toHaveBeenCalledWith(expect.objectContaining({ [COMPONENT_ID]: defaultValue })); }); + it("should not throw when maskRegex is malformed", () => { + expect(() => + renderComponent( + { maskRange: null, maskRegex: "not a /pattern/flags string [" }, + { defaultValues: { [COMPONENT_ID]: "hello" } } + ) + ).not.toThrow(); + }); + it("should render custom icons", () => { const maskIcon = "AlbumFillIcon"; const unmaskIcon = "AlbumIcon"; diff --git a/src/components/fields/masked-field/masked-field.tsx b/src/components/fields/masked-field/masked-field.tsx index df0a6e259..0c0899ec1 100644 --- a/src/components/fields/masked-field/masked-field.tsx +++ b/src/components/fields/masked-field/masked-field.tsx @@ -4,7 +4,7 @@ import * as Icons from "@lifesg/react-icons"; import React, { useEffect, useState } from "react"; import * as Yup from "yup"; import { IGenericFieldProps } from ".."; -import { TestHelper } from "../../../utils"; +import { RegexHelper, TestHelper } from "../../../utils"; import { useValidationConfig } from "../../../utils/hooks"; import { Warning } from "../../shared"; import { IMaskedFieldSchema } from "./types"; @@ -65,12 +65,11 @@ export const MaskedField = (props: IGenericFieldProps) => { // ============================================================================= const getRegex = () => { if (!maskRegex) return; - try { - const matches = maskRegex.match(/\/(.*)\/([a-z]+)?/); - return new RegExp(matches[1], matches[2]); - } catch (err) { + const regex = RegexHelper.parseMatchesPattern(maskRegex); + if (!regex) { console.warn(`invalid regex pattern: ${maskRegex}`); } + return regex; }; // ============================================================================= From d0f9cb06459eb7a8f05e777cd9c0e74b0e5cd49d Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:01:41 +0800 Subject: [PATCH 18/26] [MOL-22453][SX][asgard-0011] Strip @import/url() from locationModalStyles before applying as CSS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit locationModalStyles is a schema-authored CSS string assigned directly to an element's style.cssText. @import can load a remote stylesheet and url() can reference a remote resource — both create a network-side-channel (e.g. exfiltrating data via request timing/ querystring, or just confirming a victim rendered the form) purely from a string in the form schema, no script execution needed. Add a shared StyleHelper.sanitizeStyleString and strip both constructs before assignment. Not a full CSS parser/allowlist — the documented use case (padding/margin tweaks) needs neither construct, matching this finding's Low severity rating. Same fix applied to imageReviewModalStyles's identical sink in the next commit. --- src/__tests__/utils/style-helper.spec.ts | 26 +++++++++++++++++++ .../location-modal/location-modal.tsx | 4 +-- src/utils/index.ts | 1 + src/utils/style-helper.ts | 8 ++++++ 4 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 src/__tests__/utils/style-helper.spec.ts create mode 100644 src/utils/style-helper.ts diff --git a/src/__tests__/utils/style-helper.spec.ts b/src/__tests__/utils/style-helper.spec.ts new file mode 100644 index 000000000..015c783a4 --- /dev/null +++ b/src/__tests__/utils/style-helper.spec.ts @@ -0,0 +1,26 @@ +import { StyleHelper } from "../../utils"; + +describe("style-helper", () => { + describe("sanitizeStyleString", () => { + it("should strip @import rules", () => { + const result = StyleHelper.sanitizeStyleString( + 'padding: 1rem; @import url("https://evil.example.com/x.css");' + ); + + expect(result).not.toContain("@import"); + expect(result).toContain("padding: 1rem;"); + }); + + it("should strip url() references", () => { + const result = StyleHelper.sanitizeStyleString('background: url("https://evil.example.com/track.png");'); + + expect(result).not.toContain("url("); + }); + + it("should leave a style string with neither construct unchanged", () => { + const value = "padding: 1rem; margin: 2rem;"; + + expect(StyleHelper.sanitizeStyleString(value)).toBe(value); + }); + }); +}); diff --git a/src/components/fields/location-field/location-modal/location-modal.tsx b/src/components/fields/location-field/location-modal/location-modal.tsx index 67801e187..46c19c23b 100644 --- a/src/components/fields/location-field/location-modal/location-modal.tsx +++ b/src/components/fields/location-field/location-modal/location-modal.tsx @@ -10,7 +10,7 @@ import clsx from "clsx"; import { isEmpty } from "lodash"; import { useCallback, useEffect, useRef, useState } from "react"; import { OneMapError } from "../../../../services/onemap/types"; -import { GeoLocationHelper, TestHelper } from "../../../../utils"; +import { GeoLocationHelper, StyleHelper, TestHelper } from "../../../../utils"; import { useFieldEvent } from "../../../../utils/hooks"; import { Prompt } from "../../../shared"; import { LocationHelper } from "../location-helper"; @@ -268,7 +268,7 @@ const LocationModal = ({ useEffect(() => { if (modalBoxRef) { - modalBoxRef.style.cssText = locationModalStyles || ""; + modalBoxRef.style.cssText = StyleHelper.sanitizeStyleString(locationModalStyles || ""); } }, [locationModalStyles, modalBoxRef]); diff --git a/src/utils/index.ts b/src/utils/index.ts index d56b3cc2f..bcc85bbcd 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -10,3 +10,4 @@ export * from "./test-helper"; export * from "./types"; export * from "./prop-helper"; export * from "./regex-helper"; +export * from "./style-helper"; diff --git a/src/utils/style-helper.ts b/src/utils/style-helper.ts new file mode 100644 index 000000000..c449cd0a6 --- /dev/null +++ b/src/utils/style-helper.ts @@ -0,0 +1,8 @@ +export namespace StyleHelper { + /** strips the two concrete network-side-channel risks from a schema-authored CSS string before it's + * assigned to an element's style.cssText: @import (loads a remote stylesheet) and url() (references + * a remote resource). Not a full CSS parser/allowlist — legitimate uses of this styling hook + * (padding/margin tweaks) need neither construct. */ + export const sanitizeStyleString = (value: string): string => + value.replace(/@import[^;]*;?/gi, "").replace(/url\s*\([^)]*\)/gi, ""); +} From 3de74f425f3fc9d9e80b879058947665ea2a05cc Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:01:57 +0800 Subject: [PATCH 19/26] [MOL-22453][SX][asgard-0012] Strip @import/url() from imageReviewModalStyles before applying as CSS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same schema-authored cssText sink and same fix as asgard-0011's locationModalStyles in the previous commit — @import/url() in a CSS string assigned straight to style.cssText is a network-side-channel risk with no script execution needed. Reuses the shared StyleHelper.sanitizeStyleString from that commit. --- .../fields/image-upload/image-review/image-review.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/fields/image-upload/image-review/image-review.tsx b/src/components/fields/image-upload/image-review/image-review.tsx index 753de56a5..72bfb4770 100644 --- a/src/components/fields/image-upload/image-review/image-review.tsx +++ b/src/components/fields/image-upload/image-review/image-review.tsx @@ -14,7 +14,7 @@ import { PencilIcon } from "@lifesg/react-icons/pencil"; import { PencilStrokeIcon } from "@lifesg/react-icons/pencil-stroke"; import clsx from "clsx"; import { Suspense, lazy, useCallback, useContext, useEffect, useRef, useState } from "react"; -import { FileHelper, ImageHelper, TestHelper, generateRandomId } from "../../../../utils"; +import { FileHelper, ImageHelper, StyleHelper, TestHelper, generateRandomId } from "../../../../utils"; import { useFieldEvent, usePrevious } from "../../../../utils/hooks"; import { ImageContext } from "../image-context"; import { ImageUploadHelper } from "../image-upload-helper"; @@ -131,7 +131,7 @@ export const ImageReview = (props: IProps) => { useEffect(() => { if (modalBoxRef) { - modalBoxRef.style.cssText = imageReviewModalStyles || ""; + modalBoxRef.style.cssText = StyleHelper.sanitizeStyleString(imageReviewModalStyles || ""); } }, [imageReviewModalStyles, modalBoxRef]); From 50121919f4dfa5a1d1d19cbb689b79f9a5e5901f Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:02:13 +0800 Subject: [PATCH 20/26] [MOL-22453][SX] Bound MaskedField keystroke input length as a partial ReDoS mitigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier MaskedField commit in this series ("Use shared RegexHelper for maskRegex parsing in MaskedField") only deduplicated pattern parsing — it did not fix the ReDoS exposure, because the actual regex.test() against live keystrokes happens inside @lifesg/react-design-system's MaskedInput component, which this codebase doesn't own. That commit's message says as much explicitly; this finding was left open, not closed. This codebase does control one thing MaskedInput receives: the maxLength attribute, already derived from max/length Yup validation rules when present. Extend that derivation to default maxLength to RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH (500) whenever maskRegex is configured and no narrower max/length rule already applies. This caps the keystroke buffer length at the DOM level before MaskedInput's internal test ever runs, without needing an upstream design-system change — the same mitigation strategy already used for the other 3 ReDoS sinks, applied here via the one lever this component actually has. Not a complete fix: a pathological maskRegex can still backtrack catastrophically against inputs up to 500 characters. Genuinely closing this requires either an upstream MaskedInput change or authoring-time linting of maskRegex patterns — out of scope here. --- .../fields/masked-field/masked-field.spec.tsx | 13 +++++++++++++ src/components/fields/masked-field/masked-field.tsx | 7 ++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/__tests__/components/fields/masked-field/masked-field.spec.tsx b/src/__tests__/components/fields/masked-field/masked-field.spec.tsx index ddfec6fce..a48a7c904 100644 --- a/src/__tests__/components/fields/masked-field/masked-field.spec.tsx +++ b/src/__tests__/components/fields/masked-field/masked-field.spec.tsx @@ -1,5 +1,6 @@ import { fireEvent, screen, waitFor } from "@testing-library/react"; import { IMaskedFieldSchema } from "../../../../components/fields"; +import { RegexHelper } from "../../../../utils"; import { ERROR_MESSAGE, createRenderComponent, @@ -60,6 +61,18 @@ describe(UI_TYPE, () => { expect(getMaskedField()).toHaveAttribute("maxLength", "5"); }); + it("should default maxLength to the safe regex length bound when maskRegex is set with no max/length validation", () => { + renderComponent({ maskRange: null, maskRegex: "/^(hello)/g" }); + + expect(getMaskedField()).toHaveAttribute("maxLength", `${RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH}`); + }); + + it("should prefer an explicit max/length validation's maxLength over the maskRegex default", () => { + renderComponent({ maskRange: null, maskRegex: "/^(hello)/g", validation: [{ max: 5 }] }); + + expect(getMaskedField()).toHaveAttribute("maxLength", "5"); + }); + it("should support default value", async () => { const defaultValue = "hello"; renderComponent(undefined, { defaultValues: { [COMPONENT_ID]: defaultValue } }); diff --git a/src/components/fields/masked-field/masked-field.tsx b/src/components/fields/masked-field/masked-field.tsx index 0c0899ec1..5788c9ad7 100644 --- a/src/components/fields/masked-field/masked-field.tsx +++ b/src/components/fields/masked-field/masked-field.tsx @@ -41,10 +41,15 @@ export const MaskedField = (props: IGenericFieldProps) => { attributes.maxLength = maxRule.max; } else if (lengthRule?.length > 0) { attributes.maxLength = lengthRule.length; + } else if (maskRegex) { + // maskRegex is tested against live keystrokes inside MaskedInput itself, which this codebase + // doesn't own, so a catastrophic-backtracking pattern can't be bounded there directly. Capping + // input length here is the only mitigation available without an upstream design-system change. + attributes.maxLength = RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH; } setDerivedAttributes(attributes); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [validation]); + }, [validation, maskRegex]); useEffect(() => { if (value !== stateValue) { From c835f7e1a00e3dbcc804475c7366671a0b4175d1 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Wed, 16 Sep 2026 17:48:34 +0800 Subject: [PATCH 21/26] [MOL-22453][SX] Fix matches rule silently blocking ImageUpload submission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ImageUpload's own "matches" filename-pattern validation is handled per-file inside ImageManager (filenameMatches, tested against each image's own name) — that part works correctly and already excludes non-matching files from the submitted value. But image-upload.tsx also forwarded the raw, unfiltered validation array (including that same "matches" rule) into the generic Yup mapRules pipeline for the field's array-level schema. mapRules had no type guard on "matches" (unlike email/url/uuid, which are wrapped and warn on a type mismatch) — it added a Yup .test() that runs against the field's whole array value. Once any file actually uploads successfully, that value is an array of file objects, and RegExp.test() coerces it to a string ("[object Object]"-style) before testing — which fails almost any real pattern, rejecting the array at the form level and silently blocking the entire submission (SUBMIT_FN never fires), regardless of how much time or how many retries pass. This was already breaking any ImageUpload field configured with a matches rule as soon as a real submission was attempted — not just the mixed valid/invalid-file case that surfaced it. It predates MOL-22453 entirely and is unrelated to the security patches, but was found and is being fixed here because it deterministically blocks the pre-push hook's test run for these branches. Two-part fix: - image-upload.tsx: stop forwarding "matches" into the generic array schema at all, since ImageManager already owns that validation. - yup/helper.ts: add a type guard to mapRules's "matches" case (warn and skip for any non-string schema), matching the existing email/url/uuid pattern — defense in depth against the same class of bug on any other field, present or future. --- .../fields/image-upload/image-upload.spec.tsx | 16 ++++++++++++++++ .../frontend-engine/yup/yup-helper.spec.ts | 8 ++++++++ .../fields/image-upload/image-upload.tsx | 6 +++++- src/context-providers/yup/helper.ts | 8 ++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/__tests__/components/fields/image-upload/image-upload.spec.tsx b/src/__tests__/components/fields/image-upload/image-upload.spec.tsx index b6109ef6c..4a0383f2b 100644 --- a/src/__tests__/components/fields/image-upload/image-upload.spec.tsx +++ b/src/__tests__/components/fields/image-upload/image-upload.spec.tsx @@ -349,6 +349,22 @@ describe("image-upload", () => { await waitFor(() => expect(uploadSpy).toHaveBeenCalledTimes(1)); }); + it("should be able to submit a valid file when a matches rule is configured", async () => { + await renderComponent({ + files: [FILE_1], // "test.jpg" — lowercase alphanumeric + dot + overrideField: { validation: [{ matches: MATCHES_PATTERN, errorMessage: ERROR_MESSAGE }] }, + uploadType: "input", + }); + + await waitFor(() => expect(uploadSpy).toHaveBeenCalledTimes(1)); + await waitFor(() => fireEvent.click(getSubmitButton())); + expect(SUBMIT_FN).toHaveBeenCalledWith( + expect.objectContaining({ + field: expect.arrayContaining([expect.objectContaining({ fileName: FILE_1.name })]), + }) + ); + }); + it("should exclude invalid filename files from form submission", async () => { await renderComponent({ files: [INVALID_FILE], diff --git a/src/__tests__/components/frontend-engine/yup/yup-helper.spec.ts b/src/__tests__/components/frontend-engine/yup/yup-helper.spec.ts index c4e8f2780..0cc869fd1 100644 --- a/src/__tests__/components/frontend-engine/yup/yup-helper.spec.ts +++ b/src/__tests__/components/frontend-engine/yup/yup-helper.spec.ts @@ -257,6 +257,14 @@ describe("YupHelper", () => { expect(Date.now() - start).toBeLessThan(1000); }); + it("should skip a matches rule applied to a non-string schema instead of testing the value's string coercion", () => { + const schema = YupHelper.mapRules(Yup.array(), [{ matches: "/^[a-z]+$/", errorMessage: ERROR_MESSAGE }]); + + // an array value stringifies to something that would never satisfy a filename-shaped pattern + // (e.g. "[object Object]") — applying the rule here must not reject the value on that basis + expect(() => schema.validateSync([{ fileName: "test.jpg" }])).not.toThrow(); + }); + const generateMultipleFieldSchema = (type: "string" | "number" | "boolean" | "object" | "array") => YupHelper.buildSchema({ field1: { schema: YupHelper.mapSchemaType(type), validationRules: [] }, diff --git a/src/components/fields/image-upload/image-upload.tsx b/src/components/fields/image-upload/image-upload.tsx index 915f1ff1c..67c7bd901 100644 --- a/src/components/fields/image-upload/image-upload.tsx +++ b/src/components/fields/image-upload/image-upload.tsx @@ -155,7 +155,11 @@ export const ImageUploadInner = (props: IGenericFieldProps) ); } ), - validation + // `matches` is a per-image filename check already handled by ImageManager against each image's + // own name — it must not reach the generic Yup pipeline, which would test it against this + // field's whole array value (coerced to a string) instead of a filename, and reject valid + // submissions outright + validation?.filter((rule) => !("matches" in rule)) ); // eslint-disable-next-line react-hooks/exhaustive-deps }, [validation]); diff --git a/src/context-providers/yup/helper.ts b/src/context-providers/yup/helper.ts index 9a3612bd9..964c6189a 100644 --- a/src/context-providers/yup/helper.ts +++ b/src/context-providers/yup/helper.ts @@ -188,6 +188,14 @@ export namespace YupHelper { break; case !!rule.matches: { + // "matches" tests the field's own value as a string. Applying it to a non-string + // schema (e.g. an array field) would run RegExp.test() against that value coerced + // to a string instead — near-guaranteed to fail and reject an otherwise-valid + // submission, not a meaningful validation of anything. + if (yupSchema.type !== "string") { + console.warn(`error applying "${ruleKey}" condition to ${yupSchema.type} schema`); + break; + } const regex = RegexHelper.parseMatchesPattern(rule.matches); if (regex) { yupSchema = (yupSchema as Yup.StringSchema).test({ From 74168e15a28f270691e55d94d99dce1f0f99d19e Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Thu, 17 Sep 2026 11:43:53 +0800 Subject: [PATCH 22/26] [MOL-22453][SX] Use printf+awk to extract commit message first line, per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per PR review feedback, follow the same pattern already used in LifeSG/react-icons' equivalent workflow: printf '%s\n' | awk 'NR>1{exit};1' instead of echo -e | head -n 1. Purely a robustness nit, not a security fix — the injection vector this series' asgard-0001 commit closed (raw ${{ }} interpolation into the run: script) is unaffected either way, since HEAD_COMMIT_MSG already flows through env: before this line runs. echo -e interprets backslash escape sequences literally present in the commit message text; printf '%s' does not, which is the more correct behavior for opaque user-supplied text and matches the org's existing convention. --- .github/workflows/trigger-gitlab-pipeline.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/trigger-gitlab-pipeline.yml b/.github/workflows/trigger-gitlab-pipeline.yml index fd7f0d83d..1d4f2d638 100644 --- a/.github/workflows/trigger-gitlab-pipeline.yml +++ b/.github/workflows/trigger-gitlab-pipeline.yml @@ -24,7 +24,7 @@ jobs: run: | [[ $GITHUB_EVENT_NAME = "pull_request" ]] && BRANCH_NAME="$HEAD_REF" || BRANCH_NAME="$REF_NAME" - [[ $GITHUB_EVENT_NAME = "pull_request" ]] && COMMIT_MSG="$PR_TITLE" || COMMIT_MSG=$(echo -e "$HEAD_COMMIT_MSG" | head -n 1) + [[ $GITHUB_EVENT_NAME = "pull_request" ]] && COMMIT_MSG="$PR_TITLE" || COMMIT_MSG=$(printf '%s\n' "$HEAD_COMMIT_MSG" | awk 'NR>1{exit};1') PIPELINE_PROJECT_URL="github.com/$GITHUB_REPOSITORY.git" From ced4acdad4b2152dfde97ae3e3e7157518129e60 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Thu, 17 Sep 2026 14:08:19 +0800 Subject: [PATCH 23/26] [MOL-22453][SX] Fix Iframe postMessage origin check failing open for relative src MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The asgard-0007 fix derived the allowed origin via new URL(src) with no base URL. That throws for any relative or protocol-relative src ("/embedded/form", "//partner.example/form") — both completely ordinary ways to configure a same-origin embed. On failure, getTargetOriginFromSrc caught the error and returned null, and useIframeMessage's guard was `if (allowedOrigin && event.origin !== allowedOrigin) return;` — since null is falsy, the check never fired, silently accepting postMessage events from any origin. That's the exact vulnerability asgard-0007 was meant to close, reintroduced for any iframe using a relative src. Two-part fix: - getTargetOriginFromSrc now resolves against the embedding page (new URL(src, window.location.href)), so relative/protocol-relative src values correctly derive a real origin instead of throwing. Also rejects non-http(s) protocols (e.g. javascript:) explicitly rather than deriving a nonsensical origin from them. - useIframeMessage now distinguishes undefined (caller didn't request an origin check — used by the child-side iframe-content demo pages, which listen to their parent and don't enforce this) from null (an origin check was required but couldn't be established). Only the latter now fails closed: reject every message rather than accept every message when no valid origin could be derived. Found via external code review, not this codebase's own testing. --- .../components/custom/iframe/iframe.spec.tsx | 41 +++++++++++++++++++ src/components/custom/iframe/iframe.tsx | 11 ++++- src/utils/hooks/use-iframe-message.ts | 8 +++- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/__tests__/components/custom/iframe/iframe.spec.tsx b/src/__tests__/components/custom/iframe/iframe.spec.tsx index 559252e91..e348b041c 100644 --- a/src/__tests__/components/custom/iframe/iframe.spec.tsx +++ b/src/__tests__/components/custom/iframe/iframe.spec.tsx @@ -152,6 +152,47 @@ describe("iframe", () => { expect(SUBMIT_FN).toHaveBeenCalledWith(expect.objectContaining({ [COMPONENT_ID]: "hello world" })); }); + + it("should derive the origin for a relative src (resolved against the current page) and still ignore mismatched origins", async () => { + // new URL("/embedded/form") with no base throws; resolving against window.location.href + // (http://localhost/ in this test environment) must derive "http://localhost", not fail open + renderComponent({ src: "/embedded/form", validationTimeout: -1 }); + + sendPostMessageFromOrigin("https://attacker.example", EPostMessageEvent.SET_VALUE, "hello world"); + await waitFor(() => fireEvent.click(getSubmitButton())); + + expect(SUBMIT_FN).toHaveBeenCalledWith({}); + }); + + it("should accept a setValue postMessage matching the origin derived from a relative src", async () => { + renderComponent({ src: "/embedded/form", validationTimeout: -1 }); + + sendPostMessageFromOrigin("http://localhost", EPostMessageEvent.SET_VALUE, "hello world"); + await waitFor(() => fireEvent.click(getSubmitButton())); + + expect(SUBMIT_FN).toHaveBeenCalledWith(expect.objectContaining({ [COMPONENT_ID]: "hello world" })); + }); + + it("should derive the origin for a protocol-relative src and still ignore mismatched origins", async () => { + renderComponent({ src: "//localhost/embedded/form", validationTimeout: -1 }); + + sendPostMessageFromOrigin("https://attacker.example", EPostMessageEvent.SET_VALUE, "hello world"); + await waitFor(() => fireEvent.click(getSubmitButton())); + + expect(SUBMIT_FN).toHaveBeenCalledWith({}); + }); + + it("should reject every postMessage (fail closed) when src cannot be resolved to a valid http(s) origin", async () => { + // a non-http(s) scheme parses fine as a URL but must not be trusted as a postMessage origin — + // and critically, when no valid origin can be established, messages must be rejected by + // default, not accepted by default + renderComponent({ src: "javascript:alert(1)", validationTimeout: -1 }); + + sendPostMessageFromOrigin("https://attacker.example", EPostMessageEvent.SET_VALUE, "hello world"); + await waitFor(() => fireEvent.click(getSubmitButton())); + + expect(SUBMIT_FN).toHaveBeenCalledWith({}); + }); }); describe("load", () => { diff --git a/src/components/custom/iframe/iframe.tsx b/src/components/custom/iframe/iframe.tsx index b8a65cacb..c1217c4d8 100644 --- a/src/components/custom/iframe/iframe.tsx +++ b/src/components/custom/iframe/iframe.tsx @@ -51,8 +51,15 @@ export const Iframe = (props: IGenericCustomFieldProps) => { // ========================================================================= const getTargetOriginFromSrc = useCallback(() => { try { - const parsedUrl = new URL(src); - return `${parsedUrl.protocol}//${parsedUrl.host}`; + // resolve against the embedding page so relative/protocol-relative src values (e.g. "/embedded/form", + // "//partner.example/form") derive a real origin instead of throwing — new URL(src) with no base + // always throws for those, which previously caused the origin check to fail open (accept any origin) + // for any iframe configured with a same-origin-relative src, a completely ordinary configuration + const parsedUrl = new URL(src, window.location.href); + if (parsedUrl.protocol !== "http:" && parsedUrl.protocol !== "https:") { + return null; + } + return parsedUrl.origin; } catch (error) { console.error("Invalid URL:", error); return null; diff --git a/src/utils/hooks/use-iframe-message.ts b/src/utils/hooks/use-iframe-message.ts index 1d1f36049..e0c1a7baa 100644 --- a/src/utils/hooks/use-iframe-message.ts +++ b/src/utils/hooks/use-iframe-message.ts @@ -2,10 +2,14 @@ import { useEffect } from "react"; type MessageHandler = (event: MessageEvent<{ payload: T }>) => void; -export const useIframeMessage = (eventType: string, handler: MessageHandler, allowedOrigin?: string) => { +export const useIframeMessage = (eventType: string, handler: MessageHandler, allowedOrigin?: string | null) => { useEffect(() => { const eventHandler = (event: MessageEvent) => { - if (allowedOrigin && event.origin !== allowedOrigin) return; + // undefined = caller didn't request an origin check at all (unchanged, pre-existing behavior). + // null = caller required an origin check but couldn't establish one (e.g. an unparseable iframe + // src) — fail closed and reject every message, not fail open and accept every message. + // event.origin is always a real string set by the browser, so it can never equal null itself. + if (allowedOrigin !== undefined && event.origin !== allowedOrigin) return; if (event.data.type === eventType) { handler(event); } From 3aae54a1e9905ae0d59e8cde8f0392874c855444 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Thu, 17 Sep 2026 14:37:24 +0800 Subject: [PATCH 24/26] [MOL-22453][SX] Clamp MaskedField value length, not just the maxLength DOM attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier follow-up commit that defaulted maxLength to 500 when maskRegex is set only bounds what a user can type through the native input's own typing/paste handling. It does nothing to a value that arrives via defaultValues or a form reset: stateValue was set directly from the value prop with no length check, and handed straight to MaskedInput along with maskRegex — MaskedInput evaluates that regex against the value when rendering, regardless of what maxLength attribute happens to be sitting on the input. A long default value with a catastrophic maskRegex hangs on mount/reset with zero user interaction required, which is worse than the keystroke case the maxLength attribute actually addresses. Compute the same safe-length bound as a plain function so it's available synchronously (not just once the derivedAttributes effect has run), and apply it to stateValue itself, both on initial mount and whenever the value prop changes — not only to the DOM attribute. Found via external code review, not this codebase's own testing. Regression test added: reproduced the hang directly (process had to be killed after 15s with the fix removed) before confirming the fix resolves it. --- .../fields/masked-field/masked-field.spec.tsx | 18 +++++++++++++ .../fields/masked-field/masked-field.tsx | 25 ++++++++++++++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/__tests__/components/fields/masked-field/masked-field.spec.tsx b/src/__tests__/components/fields/masked-field/masked-field.spec.tsx index a48a7c904..a213e06b3 100644 --- a/src/__tests__/components/fields/masked-field/masked-field.spec.tsx +++ b/src/__tests__/components/fields/masked-field/masked-field.spec.tsx @@ -73,6 +73,24 @@ describe(UI_TYPE, () => { expect(getMaskedField()).toHaveAttribute("maxLength", "5"); }); + it("should not hang on a pathological maskRegex when a long value arrives via defaultValues (bypassing the maxLength DOM attribute)", () => { + // the maxLength attribute only constrains typing through the native input — a defaultValue is set + // directly on stateValue and handed to MaskedInput regardless of that attribute, so the length bound + // must also be applied when the value is set programmatically, not just derived as a DOM attribute + const maliciousValue = `${"a".repeat(600)}!`; + + const start = Date.now(); + renderComponent( + { maskRange: null, maskRegex: "/^(a+)+$/" }, + { defaultValues: { [COMPONENT_ID]: maliciousValue } } + ); + expect(Date.now() - start).toBeLessThan(1000); + + expect((getMaskedField() as HTMLInputElement).value.length).toBeLessThanOrEqual( + RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH + ); + }); + it("should support default value", async () => { const defaultValue = "hello"; renderComponent(undefined, { defaultValues: { [COMPONENT_ID]: defaultValue } }); diff --git a/src/components/fields/masked-field/masked-field.tsx b/src/components/fields/masked-field/masked-field.tsx index 5788c9ad7..4d729a73f 100644 --- a/src/components/fields/masked-field/masked-field.tsx +++ b/src/components/fields/masked-field/masked-field.tsx @@ -24,7 +24,26 @@ export const MaskedField = (props: IGenericFieldProps) => { warning, } = props; - const [stateValue, setStateValue] = useState(value || ""); + // maxLength as a DOM attribute only constrains what a user can type through the browser's native + // input handling — it does nothing to a value that arrives via defaultValues or a form reset, which + // sets stateValue (and therefore what MaskedInput evaluates maskRegex against) directly. Compute the + // same bound as a plain function (not state) so it's available synchronously wherever stateValue is + // set, not just once the derivedAttributes effect has run. + const getMaskRegexSafeLength = (): number | undefined => { + if (!maskRegex) return undefined; + const maxRule = validation?.find((rule) => "max" in rule); + const lengthRule = validation?.find((rule) => "length" in rule); + if (maxRule?.max > 0) return maxRule.max; + if (lengthRule?.length > 0) return lengthRule.length; + return RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH; + }; + + const clampValue = (val: string | undefined): string => { + const safeLength = getMaskRegexSafeLength(); + return safeLength !== undefined ? (val || "").slice(0, safeLength) : val || ""; + }; + + const [stateValue, setStateValue] = useState(() => clampValue(value)); const [derivedAttributes, setDerivedAttributes] = useState({}); const { setFieldValidationConfig } = useValidationConfig(); @@ -52,9 +71,7 @@ export const MaskedField = (props: IGenericFieldProps) => { }, [validation, maskRegex]); useEffect(() => { - if (value !== stateValue) { - setStateValue(value || ""); - } + setStateValue(clampValue(value)); // eslint-disable-next-line react-hooks/exhaustive-deps }, [value]); From 6a091ded1497558b0673a95947caa0dd264141a8 Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Thu, 17 Sep 2026 15:28:43 +0800 Subject: [PATCH 25/26] [MOL-22453][SX] Add regression test documenting the ReDoS bound's real limitation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every existing "should not hang" test for the 4 ReDoS sinks uses a 501+/600+ character malicious input — all that proves is the length short-circuit itself is fast, not that the bound protects against catastrophic backtracking. The classic pattern these fixes cite (/^(a+)+$/) already costs real, measurable time well under the 500-char threshold: independently measured at 20 chars/21ms, 25 chars/160-800ms, 30 chars/~5s. The suite had no test exercising that actual danger zone. Add one canonical test in regex-helper.spec.ts (the shared utility 3 of the 4 sinks call directly) using a 25-character near-match, calibrated to complete in under a second on this measurement while staying nowhere near the safe length bound — demonstrating the input is not short-circuited and the underlying regex actually runs, unlike every existing 501+/600+ char test. Generous 20s per-test timeout to tolerate normal measurement variance across CI hardware without flaking, not because a hang is expected. Found via external code review, not this codebase's own testing. --- src/__tests__/utils/regex-helper.spec.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/__tests__/utils/regex-helper.spec.ts b/src/__tests__/utils/regex-helper.spec.ts index b23091cff..a3b868a03 100644 --- a/src/__tests__/utils/regex-helper.spec.ts +++ b/src/__tests__/utils/regex-helper.spec.ts @@ -33,11 +33,25 @@ describe("regex-helper", () => { }); it("should not hang and should return false when value exceeds the safe length bound, even for a pathological pattern", () => { + // this only proves the length short-circuit itself is fast — it does not prove the bound + // protects against catastrophic backtracking in general. See the test below. const maliciousValue = `${"a".repeat(RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH + 1)}!`; const start = Date.now(); expect(RegexHelper.safeTestRegex(/^(a+)+$/, maliciousValue)).toBe(false); expect(Date.now() - start).toBeLessThan(1000); }); + + it("known limitation: a pathological pattern well below the safe length bound is not short-circuited and still backtracks catastrophically", () => { + // documents that the 500-char bound does not meaningfully protect against the classic + // catastrophic-backtracking shape it was written for — that shape already costs real, + // measurable time well under the bound (this length was independently measured at + // 200-800ms; the trend is exponential, so 30+ chars is already multi-second). This test's + // generous timeout exists to tolerate that measured cost, not because a hang is expected — + // see MOL-22453-release-notes.md for the full measurement and the accepted-limitation writeup. + const nearMatch = `${"a".repeat(25)}!`; + + expect(RegexHelper.safeTestRegex(/^(a+)+$/, nearMatch)).toBe(false); + }, 20000); }); }); From d687d9b1a764746fcccdaf0ab6b2d729ff0e692c Mon Sep 17 00:00:00 2001 From: "Chen Shengxi (Dave)" Date: Thu, 17 Sep 2026 19:16:16 +0800 Subject: [PATCH 26/26] [MOL-22453][SX] Clamp MaskedField at render time and reject oversized values MaskedInput keeps its own internal raw-value state instead of always deriving it from the value prop. When maskRegex newly appears while that internal state still holds a long pre-existing value, it re-masks against its stale internal value, not whatever (already-clamped) value we pass on that render, reintroducing the catastrophic-backtracking hang regardless of our own clamping. Force a remount by keying on maskRegex so the fresh instance always initialises from the current clamped value, and derive the displayed value at render time rather than only in an effect that runs after the render has already committed. Also add an implicit max-length validation constraint when maskRegex is set with no explicit author max/length rule, so an oversized programmatic value (defaultValues, setValue, reset) is rejected with a field error instead of silently displaying truncated while the full, uncapped value remains in form state and gets submitted as-is. --- .../fields/masked-field/masked-field.spec.tsx | 60 ++++++++++++++++++- .../fields/masked-field/masked-field.tsx | 44 +++++++++++--- src/components/shared/error-messages.tsx | 3 + 3 files changed, 99 insertions(+), 8 deletions(-) diff --git a/src/__tests__/components/fields/masked-field/masked-field.spec.tsx b/src/__tests__/components/fields/masked-field/masked-field.spec.tsx index a213e06b3..9fdf87f3c 100644 --- a/src/__tests__/components/fields/masked-field/masked-field.spec.tsx +++ b/src/__tests__/components/fields/masked-field/masked-field.spec.tsx @@ -1,5 +1,8 @@ -import { fireEvent, screen, waitFor } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { FrontendEngine } from "../../../../components"; import { IMaskedFieldSchema } from "../../../../components/fields"; +import { ERROR_MESSAGES } from "../../../../components/shared"; +import { IFrontendEngineData } from "../../../../components/types"; import { RegexHelper } from "../../../../utils"; import { ERROR_MESSAGE, @@ -91,6 +94,61 @@ describe(UI_TYPE, () => { ); }); + it("should clamp an already-loaded long value at render time when maskRegex changes at runtime, not only when the value itself changes", () => { + // the effect that clamps stateValue is keyed on [safeLength, value] — but the render that + // introduces a new maskRegex happens before that effect runs. If MaskedInput were handed the + // unclamped stateValue on that render, a pathological pattern could reach it before the effect + // has any chance to shorten the value, so the render itself must derive a clamped value directly + const maliciousValue = `${"a".repeat(600)}!`; + const withoutMaskRegex: IFrontendEngineData = JSON.parse(JSON.stringify(schema)); + Object.assign(withoutMaskRegex, { defaultValues: { [COMPONENT_ID]: maliciousValue } }); + const { rerender } = render(); + + const withMaskRegex: IFrontendEngineData = JSON.parse(JSON.stringify(withoutMaskRegex)); + Object.assign(withMaskRegex.sections.section.children[COMPONENT_ID] as object, { + maskRange: null, + maskRegex: "/^(a+)+$/", + }); + + const start = Date.now(); + rerender(); + expect(Date.now() - start).toBeLessThan(1000); + + expect((getMaskedField() as HTMLInputElement).value.length).toBeLessThanOrEqual( + RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH + ); + }); + + it("should reject an oversized programmatic value with a validation error when maskRegex is set but no explicit max/length rule governs the length", async () => { + const oversizedValue = "a".repeat(RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH + 1); + renderComponent( + { maskRange: null, maskRegex: "/^(hello)/g" }, + { defaultValues: { [COMPONENT_ID]: oversizedValue } } + ); + + await waitFor(() => fireEvent.click(getSubmitButton())); + + expect( + getErrorMessage( + false, + ERROR_MESSAGES.MASKED_FIELD.VALUE_TOO_LONG(RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH) + ) + ).toBeInTheDocument(); + expect(SUBMIT_FN).not.toHaveBeenCalled(); + }); + + it("should not reject an oversized value when an explicit max validation rule already permits that length", async () => { + const value = "a".repeat(RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH + 1); + renderComponent( + { maskRange: null, maskRegex: "/^(hello)/g", validation: [{ max: 1000 }] }, + { defaultValues: { [COMPONENT_ID]: value } } + ); + + await waitFor(() => fireEvent.click(getSubmitButton())); + + expect(SUBMIT_FN).toHaveBeenCalledWith(expect.objectContaining({ [COMPONENT_ID]: value })); + }); + it("should support default value", async () => { const defaultValue = "hello"; renderComponent(undefined, { defaultValues: { [COMPONENT_ID]: defaultValue } }); diff --git a/src/components/fields/masked-field/masked-field.tsx b/src/components/fields/masked-field/masked-field.tsx index 4d729a73f..7088c9410 100644 --- a/src/components/fields/masked-field/masked-field.tsx +++ b/src/components/fields/masked-field/masked-field.tsx @@ -6,7 +6,7 @@ import * as Yup from "yup"; import { IGenericFieldProps } from ".."; import { RegexHelper, TestHelper } from "../../../utils"; import { useValidationConfig } from "../../../utils/hooks"; -import { Warning } from "../../shared"; +import { ERROR_MESSAGES, Warning } from "../../shared"; import { IMaskedFieldSchema } from "./types"; export const MaskedField = (props: IGenericFieldProps) => { @@ -38,23 +38,45 @@ export const MaskedField = (props: IGenericFieldProps) => { return RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH; }; + // recomputed every render (not memoized) so it always reflects the maskRegex/validation in effect + // for *this* render, not a stale value from before an async effect has caught up + const safeLength = getMaskRegexSafeLength(); + const clampValue = (val: string | undefined): string => { - const safeLength = getMaskRegexSafeLength(); - return safeLength !== undefined ? (val || "").slice(0, safeLength) : val || ""; + const stringVal = val ?? ""; + return safeLength !== undefined ? stringVal.slice(0, safeLength) : stringVal; }; const [stateValue, setStateValue] = useState(() => clampValue(value)); const [derivedAttributes, setDerivedAttributes] = useState({}); const { setFieldValidationConfig } = useValidationConfig(); + // clamped at render time, not just in the effect below — an effect only runs after a render has + // already committed, so if maskRegex changes while stateValue is still a long, pre-existing value, + // the unclamped stateValue would otherwise reach MaskedInput's own regex-driven masking on that + // render, before the effect gets a chance to shorten it + const displayedValue = clampValue(stateValue); + // ============================================================================= // EFFECTS // ============================================================================= useEffect(() => { - setFieldValidationConfig(id, Yup.string(), validation); - const maxRule = validation?.find((rule) => "max" in rule); const lengthRule = validation?.find((rule) => "length" in rule); + + let schema = Yup.string(); + if (maskRegex && !maxRule && !lengthRule) { + // no author-specified max/length exists to validate against, so without this the implicit + // safe-length bound used for clamping/masking is never actually enforced as a real + // validation error - an oversized programmatic value would just be silently clamped for + // display while the full value remains in form state and gets submitted as-is + schema = schema.max( + RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH, + ERROR_MESSAGES.MASKED_FIELD.VALUE_TOO_LONG(RegexHelper.MAX_SAFE_PATTERN_INPUT_LENGTH) + ); + } + setFieldValidationConfig(id, schema, validation); + const attributes = { ...derivedAttributes }; if (maxRule?.max > 0) { attributes.maxLength = maxRule.max; @@ -73,7 +95,7 @@ export const MaskedField = (props: IGenericFieldProps) => { useEffect(() => { setStateValue(clampValue(value)); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [value]); + }, [safeLength, value]); // ============================================================================= // EVENT HANDLERS @@ -109,12 +131,20 @@ export const MaskedField = (props: IGenericFieldProps) => { `Value exceeds the maximum allowed length of ${maxLength} characters.`, + }, ARRAY_FIELD: { INVALID: "One or more of the sections is incomplete", REQUIRED: "At least one section must be filled in",