diff --git a/editor.planx.uk/package.json b/editor.planx.uk/package.json index 0a41be1b8c..71e3bd9d23 100644 --- a/editor.planx.uk/package.json +++ b/editor.planx.uk/package.json @@ -45,6 +45,7 @@ "classnames": "^2.3.2", "core-js": "^3.31.0", "date-fns": "^2.30.0", + "dompurify": "^3.0.6", "dotenv": "^16.3.1", "formik": "^2.4.2", "graphql": "^16.8.1", @@ -116,6 +117,7 @@ "@testing-library/jest-dom": "^5.16.5", "@testing-library/react": "^13.4.0", "@testing-library/user-event": "^14.4.3", + "@types/dompurify": "^3.0.5", "@types/draft-js": "^0.11.12", "@types/jest": "^27.5.2", "@types/jest-axe": "^3.5.5", diff --git a/editor.planx.uk/pnpm-lock.yaml b/editor.planx.uk/pnpm-lock.yaml index f51f8eb03c..7221962afb 100644 --- a/editor.planx.uk/pnpm-lock.yaml +++ b/editor.planx.uk/pnpm-lock.yaml @@ -137,6 +137,9 @@ dependencies: date-fns: specifier: ^2.30.0 version: 2.30.0 + dompurify: + specifier: ^3.0.6 + version: 3.0.6 dotenv: specifier: ^16.3.1 version: 16.3.1 @@ -346,6 +349,9 @@ devDependencies: '@testing-library/user-event': specifier: ^14.4.3 version: 14.4.3(@testing-library/dom@8.20.1) + '@types/dompurify': + specifier: ^3.0.5 + version: 3.0.5 '@types/draft-js': specifier: ^0.11.12 version: 0.11.12 @@ -4641,7 +4647,7 @@ packages: '@babel/runtime': 7.23.2 '@emotion/is-prop-valid': 1.2.1 '@mui/types': 7.2.9(@types/react@18.2.20) - '@mui/utils': 5.14.5(react@18.2.0) + '@mui/utils': 5.14.18(@types/react@18.2.20)(react@18.2.0) '@popperjs/core': 2.11.8 '@types/react': 18.2.20 clsx: 2.0.0 @@ -7333,6 +7339,12 @@ packages: resolution: {integrity: sha512-w5jZ0ee+HaPOaX25X2/2oGR/7rgAQSYII7X7pp0m9KgBfMP7uKfMfTvcpl5Dj+eDBbpxKGiqE+flqDr6XTd2RA==} dev: true + /@types/dompurify@3.0.5: + resolution: {integrity: sha512-1Wg0g3BtQF7sSb27fJQAKck1HECM6zV1EB66j8JH9i3LCjYabJa0FSdiSgsD5K/RbrsR0SiraKacLB+T8ZVYAg==} + dependencies: + '@types/trusted-types': 2.0.6 + dev: true + /@types/draft-js@0.11.12: resolution: {integrity: sha512-J/e4QYz8wCXvPpiCaiKcJrtLo65px4nnfFVZ/0EKHoKnQ4nWdzXwGHOQLIePAJM+Ho4V9/mb4Nhw4v/08y98jQ==} dependencies: @@ -9643,7 +9655,7 @@ packages: dev: false /concat-map@0.0.1: - resolution: {integrity: sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg==} + resolution: {integrity: sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=} /concat-stream@1.6.2: resolution: {integrity: sha512-27HBghJxjiZtIk3Ycvn/4kbJk/1uZuJFfuPEns6LaEvpvG1f0hTea8lilrouyo9mVc2GWdcEZ8OLoGmSADlrCw==} @@ -10497,6 +10509,10 @@ packages: dev: false optional: true + /dompurify@3.0.6: + resolution: {integrity: sha512-ilkD8YEnnGh1zJ240uJsW7AzE+2qpbOUYjacomn3AvJ6J4JhKGSZ2nh4wUIXPZrEPppaCLx5jFe8T89Rk8tQ7w==} + dev: false + /domutils@1.7.0: resolution: {integrity: sha512-Lgd2XcJ/NjEw+7tFvfKxOzCYKZsdct5lczQ2ZaQY8Djz7pfAD3Gbp8ySJWtreII/vDlMVmxwa6pHmdxIYgttDg==} dependencies: @@ -13665,7 +13681,7 @@ packages: resolution: {integrity: sha512-Qczi5xnTNjkhcIB0Yy75Txt+Ez51xdhOxsukN7awzq2auZQGPHcQrJ623PZj0ECDEMOk2soxWx05EXdXGd1CbA==} engines: {node: ^10.13.0 || ^12.13.0 || ^14.15.0 || >=15.0.0} dependencies: - chalk: 4.1.0 + chalk: 4.1.2 jest-diff: 27.5.1 jest-get-type: 27.5.1 pretty-format: 27.5.1 diff --git a/editor.planx.uk/src/@planx/graph/__tests__/add.test.ts b/editor.planx.uk/src/@planx/graph/__tests__/add.test.ts index d6dd86c546..15e1eaa2f5 100644 --- a/editor.planx.uk/src/@planx/graph/__tests__/add.test.ts +++ b/editor.planx.uk/src/@planx/graph/__tests__/add.test.ts @@ -137,6 +137,33 @@ test("ignores empty values", () => { ]); }); +test("sanitizes unsafe input", () => { + const [graph, ops] = add({ + id: "test", + type: 100, + data: { + text: "efef", + description: "
Test
", + }, + })({}); + expect(graph).toEqual({ + _root: { + edges: ["test"], + }, + test: { + type: 100, + data: { + text: "efef", + description: `Test
`, + }, + }, + }); + expect(ops).toEqual([ + { oi: { edges: ["test"] }, p: ["_root"] }, + { oi: { data: { text: "efef", description: `Test
` }, type: 100 }, p: ["test"] }, + ]); +}); + test("empty graph", () => { const [graph, ops] = add({ id: "a" })(); expect(graph).toEqual({ diff --git a/editor.planx.uk/src/@planx/graph/__tests__/sanitize.test.ts b/editor.planx.uk/src/@planx/graph/__tests__/sanitize.test.ts new file mode 100644 index 0000000000..ea0cb8de68 --- /dev/null +++ b/editor.planx.uk/src/@planx/graph/__tests__/sanitize.test.ts @@ -0,0 +1,13 @@ +import { sanitize } from ".."; + +test("sanitizes string", () => { + const bad = "Test
"; + expect(sanitize(bad)).toEqual(`Test
`); +}); + +test("sanitizes data object values", () => { + const badData = { + description: "Test
", + }; + expect(sanitize(badData)).toEqual({ description: `Test
` }); +}); diff --git a/editor.planx.uk/src/@planx/graph/__tests__/update.test.ts b/editor.planx.uk/src/@planx/graph/__tests__/update.test.ts index 7c5e1dd1a0..ff2a581e24 100644 --- a/editor.planx.uk/src/@planx/graph/__tests__/update.test.ts +++ b/editor.planx.uk/src/@planx/graph/__tests__/update.test.ts @@ -26,6 +26,29 @@ describe("updating", () => { expect(ops).toEqual([]); }); + test("doesn't save unsafe data", () => { + const [graph, ops] = update("a", { + description: "Test
", + })({ + a: { + data: { + text: "efef", + }, + }, + }); + + expect(graph).toEqual({ + a: { + data: { + text: "efef", + description: `Test
`, + }, + }, + }); + + expect(ops).toEqual([{ oi: `Test
`, p: ["a", "data", "description"] }]); + }); + test("add a field to a without affecting existing data", () => { const [graph, ops] = update("a", { foo: "bar" })({ a: { diff --git a/editor.planx.uk/src/@planx/graph/index.ts b/editor.planx.uk/src/@planx/graph/index.ts index a45bc780b6..34c042b630 100644 --- a/editor.planx.uk/src/@planx/graph/index.ts +++ b/editor.planx.uk/src/@planx/graph/index.ts @@ -1,7 +1,8 @@ import { TYPES } from "@planx/components/types"; +import DOMPurify from "dompurify"; import { enablePatches, produceWithPatches } from "immer"; -import { isEqual } from "lodash"; import difference from "lodash/difference"; +import isEqual from "lodash/isEqual"; import trim from "lodash/trim"; import zip from "lodash/zip"; import { customAlphabet } from "nanoid-good"; @@ -44,12 +45,14 @@ const isSectionNodeType = (id: string, graph: Graph): boolean => const isExternalPortalNodeType = (id: string, graph: Graph): boolean => graph[id]?.type === TYPES.ExternalPortal; -const sanitize = (x: any) => { +export const sanitize = (x: any) => { if ((x && typeof x === "string") || x instanceof String) { + x = DOMPurify.sanitize(x as string); return trim(x.replace(/[\u200B-\u200D\uFEFF↵]/g, "")); } else if ((x && typeof x === "object") || x instanceof Object) { return Object.entries(x).reduce((acc, [k, v]) => { v = sanitize(v); + acc[k] = v; if ( !isSomething(v) || (typeof v === "object" && Object.keys(v as object).length === 0)