fix: sanitize sharedb node data using dompurify in sharedb.planx.uk

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",

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: "<p>Test<img src=x onerror=prompt('Stored XSS')/></p>",
+    },
+  })({});
+  expect(graph).toEqual({
+    _root: {
+      edges: ["test"],
+    },
+    test: {
+      type: 100,
+      data: {
+        text: "efef",
+        description: `<p>Test<img src="x"></p>`,
+      },
+    },
+  });
+  expect(ops).toEqual([
+    { oi: { edges: ["test"] }, p: ["_root"] },
+    { oi: { data: { text: "efef", description: `<p>Test<img src="x"></p>` }, type: 100 }, p: ["test"] },
+  ]);
+});
+
 test("empty graph", () => {
   const [graph, ops] = add({ id: "a" })();
   expect(graph).toEqual({

editor.planx.uk/src/@planx/graph/__tests__/sanitize.test.ts

@@ -0,0 +1,13 @@
+import { sanitize } from "..";
+
+test("sanitizes string", () => {
+  const bad = "<p>Test<img src=x onerror=prompt('Stored XSS')/></p>";
+  expect(sanitize(bad)).toEqual(`<p>Test<img src="x"></p>`);
+});
+
+test("sanitizes data object values", () => {
+  const badData = {
+    description: "<p>Test<img src=x onerror=prompt('Stored XSS')/></p>",
+  };
+  expect(sanitize(badData)).toEqual({ description: `<p>Test<img src="x"></p>` });
+});

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: "<p>Test<img src=x onerror=prompt('Stored XSS')/></p>",
+      })({
+        a: {
+          data: {
+            text: "efef",
+          },
+        },
+      });
+
+      expect(graph).toEqual({
+        a: {
+          data: {
+            text: "efef",
+            description: `<p>Test<img src="x"></p>`,
+          },
+        },
+      });
+
+      expect(ops).toEqual([{ oi: `<p>Test<img src="x"></p>`, p: ["a", "data", "description"] }]);
+    });
+
     test("add a field to a without affecting existing data", () => {
       const [graph, ops] = update("a", { foo: "bar" })({
         a: {

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)

sharedb.planx.uk/package.json

@@ -4,6 +4,7 @@
   "private": true,
   "dependencies": {
     "@teamwork/websocket-json-stream": "^2.0.0",
+    "dompurify": "^3.0.6",
     "jsonwebtoken": "^8.5.1",
     "pg": "^8.11.3",
     "sharedb": "^3.3.1",

sharedb.planx.uk/sharedb-postgresql.js

@@ -1,5 +1,6 @@
 const { Pool } = require("pg");
 const { DB } = require("sharedb");
+const DOMPurify = require("dompurify");
 
 function PostgresDB(options) {
   if (!(this instanceof PostgresDB)) {
@@ -25,6 +26,22 @@ function rollback(client, done) {
   client.query("ROLLBACK", (err) => done(err));
 }
 
+// Also see editor.planx.uk/src/@planx/graph/index.ts
+// This is a simplified implementation that only handles purification of unsafe values, it does not handle empty values
+function sanitize(x) {
+  if ((x && typeof x === "string") || x instanceof String) {
+    return DOMPurify.sanitize(x);
+  } else if ((x && typeof x === "object") || x instanceof Object) {
+    return Object.entries(x).reduce((acc, [k, v]) => {
+      v = sanitize(v);
+      acc[k] = v;
+      return acc;
+    }, x);
+  } else {
+    return x;
+  }
+}
+
 // Persists an op and snapshot if it is for the next version. Calls back with
 // callback(err, succeeded)
 PostgresDB.prototype.commit = function (
@@ -35,6 +52,10 @@ PostgresDB.prototype.commit = function (
   _options,
   callback
 ) {
+  // Remove any unsafe values from this operation before committing
+  op = sanitize(op);
+  console.log('sanitised op', op);
+
   const { uId: actorId } = op.m;
 
   /*