diff --git a/.github/workflows/trigger-gitlab-pipeline.yml b/.github/workflows/trigger-gitlab-pipeline.yml index babc714f3..1d4f2d638 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=$(printf '%s\n' "$HEAD_COMMIT_MSG" | awk 'NR>1{exit};1') PIPELINE_PROJECT_URL="github.com/$GITHUB_REPOSITORY.git" 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" 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/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", 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" }, 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/__tests__/components/custom/iframe/iframe.spec.tsx b/src/__tests__/components/custom/iframe/iframe.spec.tsx index 0f6567afd..e348b041c 100644 --- a/src/__tests__/components/custom/iframe/iframe.spec.tsx +++ b/src/__tests__/components/custom/iframe/iframe.spec.tsx @@ -130,6 +130,71 @@ 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" })); + }); + + 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", () => { it("should fire a loading event when iframe starts loading", () => { const testFn = jest.fn(); 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/__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/__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/__tests__/components/fields/file-upload/file-upload.spec.tsx b/src/__tests__/components/fields/file-upload/file-upload.spec.tsx index ad9de21c5..cb8913d2a 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,27 @@ 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(); + await waitFor(() => { + expect(screen.getAllByText(ERROR_MESSAGES.UPLOAD().GENERIC).length).toBe(2); + }); + }); + it("should support default value without dataURL and fileUrl", async () => { jest.spyOn(AxiosApiClient.prototype, "get").mockResolvedValue(FILE_1); await renderComponent({ @@ -268,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 }], @@ -295,6 +338,7 @@ describe(UI_TYPE, () => { }, }; await renderComponent({ + overrideField: { trustProvidedFileMetadata: true }, overrideSchema: { defaultValues: { [COMPONENT_ID]: [{ fileId: FILE_1.name, fileName: FILE_1.name, uploadResponse }], @@ -311,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: { @@ -338,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 }], @@ -365,6 +434,7 @@ describe(UI_TYPE, () => { }, }; await renderComponent({ + overrideField: { trustProvidedFileMetadata: true }, overrideSchema: { defaultValues: { [COMPONENT_ID]: [{ fileId: FILE_1.name, fileName: FILE_1.name, uploadResponse }], @@ -595,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 }], }, @@ -646,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/__tests__/components/fields/image-upload/image-upload.spec.tsx b/src/__tests__/components/fields/image-upload/image-upload.spec.tsx index c4f58197e..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], @@ -380,6 +396,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/__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/__tests__/components/fields/masked-field/masked-field.spec.tsx b/src/__tests__/components/fields/masked-field/masked-field.spec.tsx index 573effd8f..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,9 @@ -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, createRenderComponent, @@ -60,6 +64,91 @@ 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 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 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 } }); @@ -99,6 +188,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/__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/__tests__/components/frontend-engine/yup/yup-helper.spec.ts b/src/__tests__/components/frontend-engine/yup/yup-helper.spec.ts index 2f4b4a75d..0cc869fd1 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,23 @@ 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); + }); + + 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/__tests__/utils/regex-helper.spec.ts b/src/__tests__/utils/regex-helper.spec.ts new file mode 100644 index 000000000..a3b868a03 --- /dev/null +++ b/src/__tests__/utils/regex-helper.spec.ts @@ -0,0 +1,57 @@ +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", () => { + // 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); + }); +}); 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/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} )} diff --git a/src/components/custom/iframe/iframe.tsx b/src/components/custom/iframe/iframe.tsx index d26eccbf8..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; @@ -130,11 +137,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 +157,8 @@ export const Iframe = (props: IGenericCustomFieldProps) => { width: e.data.payload?.width, height: e.data.payload?.height, }); - }, []) + }, []), + allowedOrigin ); useIframeMessage( @@ -154,7 +168,8 @@ export const Iframe = (props: IGenericCustomFieldProps) => { formContext.setValue(id, e.data.payload, { shouldDirty: true }); }, [formContext, id] - ) + ), + allowedOrigin ); useIframeMessage( @@ -169,14 +184,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/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 ( 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 => { 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; } diff --git a/src/components/fields/file-upload/file-upload-manager.ts b/src/components/fields/file-upload/file-upload-manager.ts index e1d7babfb..677de331d 100644 --- a/src/components/fields/file-upload/file-upload-manager.ts +++ b/src/components/fields/file-upload/file-upload-manager.ts @@ -15,12 +15,14 @@ import { } from "./types"; interface IProps { + allowedFileOrigins?: string[] | undefined; compressImages: boolean; fileTypeRule: IFileUploadValidationRule; fileExtensionRule: IFileUploadValidationRule; hideThumbnail?: boolean | undefined; id: string; maxFileSizeRule: IFileUploadValidationRule; + trustProvidedFileMetadata: boolean; upload: IFileUploadSchema["uploadOnAddingFile"]; uploadRule: IFileUploadValidationRule; value: IFileUploadValue[]; @@ -28,17 +30,36 @@ 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, hideThumbnail, id, maxFileSizeRule, + trustProvidedFileMetadata, upload, uploadRule, value, @@ -284,7 +305,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); @@ -294,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 45524eb33..15c00285c 100644 --- a/src/components/fields/file-upload/file-upload.tsx +++ b/src/components/fields/file-upload/file-upload.tsx @@ -33,10 +33,12 @@ export const FileUploadInner = (props: IGenericFieldProps) => isTouched, value, schema: { + allowedFileOrigins, compressImages, description, hideThumbnail, label, + trustProvidedFileMetadata, uploadOnAddingFile, validation, warning: schemaWarning, @@ -312,12 +314,14 @@ 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; + /** 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 { 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 { 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]); 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/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/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, diff --git a/src/components/fields/masked-field/masked-field.tsx b/src/components/fields/masked-field/masked-field.tsx index df0a6e259..7088c9410 100644 --- a/src/components/fields/masked-field/masked-field.tsx +++ b/src/components/fields/masked-field/masked-field.tsx @@ -4,9 +4,9 @@ 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 { ERROR_MESSAGES, Warning } from "../../shared"; import { IMaskedFieldSchema } from "./types"; export const MaskedField = (props: IGenericFieldProps) => { @@ -24,34 +24,78 @@ 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; + }; + + // 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 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; } 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) { - setStateValue(value || ""); - } + setStateValue(clampValue(value)); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [value]); + }, [safeLength, value]); // ============================================================================= // EVENT HANDLERS @@ -65,12 +109,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; }; // ============================================================================= @@ -88,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", 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 */ diff --git a/src/context-providers/yup/helper.ts b/src/context-providers/yup/helper.ts index 90f147efd..964c6189a 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,23 @@ 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) { + // "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({ + 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/hooks/use-iframe-message.ts b/src/utils/hooks/use-iframe-message.ts index 13d673e6a..e0c1a7baa 100644 --- a/src/utils/hooks/use-iframe-message.ts +++ b/src/utils/hooks/use-iframe-message.ts @@ -2,9 +2,14 @@ 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 | null) => { useEffect(() => { const eventHandler = (event: MessageEvent) => { + // 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); } @@ -17,5 +22,5 @@ export const useIframeMessage = (eventType: string, handler: MessageHandler { window.removeEventListener("message", eventHandler); }; - }, [eventType, handler]); + }, [eventType, handler, allowedOrigin]); }; diff --git a/src/utils/index.ts b/src/utils/index.ts index 89bd1b231..bcc85bbcd 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -9,3 +9,5 @@ export * from "./object-helper"; export * from "./test-helper"; export * from "./types"; export * from "./prop-helper"; +export * from "./regex-helper"; +export * from "./style-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); + }; +} 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, ""); +}