fix: Add additional validation to table attributes (#12156)

* fix: Add additional validation to table attributes

* fix: Widen isValidCellMarks predicate and test 4-digit hex

* Additional tests

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

shared/editor/lib/table.test.ts

@@ -0,0 +1,167 @@
+import { Schema } from "prosemirror-model";
+import { isValidCellAlignment, isValidCellMarks } from "./table";
+
+const schema = new Schema({
+  nodes: {
+    doc: { content: "text*" },
+    text: {},
+  },
+  marks: {
+    background: {
+      attrs: { color: {} },
+    },
+    strong: {},
+    em: {},
+  },
+});
+
+describe("isValidCellAlignment", () => {
+  it("accepts null", () => {
+    expect(isValidCellAlignment(null)).toBe(true);
+  });
+
+  it("accepts the three allowed alignment strings", () => {
+    expect(isValidCellAlignment("left")).toBe(true);
+    expect(isValidCellAlignment("center")).toBe(true);
+    expect(isValidCellAlignment("right")).toBe(true);
+  });
+
+  it("rejects arbitrary strings", () => {
+    expect(isValidCellAlignment("justify")).toBe(false);
+    expect(isValidCellAlignment("LEFT")).toBe(false);
+    expect(isValidCellAlignment("")).toBe(false);
+  });
+
+  it("rejects CSS injection payloads", () => {
+    expect(
+      isValidCellAlignment(
+        "left; background-image: url(https://evil.example/exfil)"
+      )
+    ).toBe(false);
+    expect(isValidCellAlignment("left; background: red")).toBe(false);
+  });
+
+  it("rejects non-string non-null values", () => {
+    expect(isValidCellAlignment(undefined)).toBe(false);
+    expect(isValidCellAlignment(0)).toBe(false);
+    expect(isValidCellAlignment({})).toBe(false);
+    expect(isValidCellAlignment([])).toBe(false);
+  });
+});
+
+describe("isValidCellMarks", () => {
+  it("accepts undefined and null", () => {
+    expect(isValidCellMarks(undefined)).toBe(true);
+    expect(isValidCellMarks(null)).toBe(true);
+  });
+
+  it("accepts an empty array", () => {
+    expect(isValidCellMarks([])).toBe(true);
+  });
+
+  it("accepts a background mark with a valid 6-digit hex color", () => {
+    expect(
+      isValidCellMarks(
+        [{ type: "background", attrs: { color: "#FDEA9B" } }],
+        schema
+      )
+    ).toBe(true);
+  });
+
+  it("accepts a background mark with a valid 8-digit hex color", () => {
+    expect(
+      isValidCellMarks(
+        [{ type: "background", attrs: { color: "#FDEA9BB3" } }],
+        schema
+      )
+    ).toBe(true);
+  });
+
+  it("accepts a background mark with a valid 3-digit hex color", () => {
+    expect(
+      isValidCellMarks(
+        [{ type: "background", attrs: { color: "#fff" } }],
+        schema
+      )
+    ).toBe(true);
+  });
+
+  it("accepts a background mark with a valid 4-digit hex color", () => {
+    expect(
+      isValidCellMarks(
+        [{ type: "background", attrs: { color: "#fff8" } }],
+        schema
+      )
+    ).toBe(true);
+  });
+
+  it("rejects a background mark with a malformed 4-digit hex color", () => {
+    expect(
+      isValidCellMarks(
+        [{ type: "background", attrs: { color: "#fffz" } }],
+        schema
+      )
+    ).toBe(false);
+  });
+
+  it("rejects a background mark with a non-hex color", () => {
+    expect(
+      isValidCellMarks(
+        [{ type: "background", attrs: { color: "red" } }],
+        schema
+      )
+    ).toBe(false);
+  });
+
+  it("rejects a background mark carrying a CSS injection payload", () => {
+    expect(
+      isValidCellMarks(
+        [
+          {
+            type: "background",
+            attrs: {
+              color: "#fff; background-image: url(https://evil.example)",
+            },
+          },
+        ],
+        schema
+      )
+    ).toBe(false);
+  });
+
+  it("rejects a background mark with a missing color attr", () => {
+    expect(isValidCellMarks([{ type: "background", attrs: {} }], schema)).toBe(
+      false
+    );
+    expect(isValidCellMarks([{ type: "background" }], schema)).toBe(false);
+  });
+
+  it("rejects mark types that are not registered in the schema", () => {
+    expect(isValidCellMarks([{ type: "not_a_mark" }], schema)).toBe(false);
+  });
+
+  it("does not check schema registration when no schema is provided", () => {
+    expect(isValidCellMarks([{ type: "anything_goes" }])).toBe(true);
+  });
+
+  it("rejects non-array values", () => {
+    expect(isValidCellMarks("marks")).toBe(false);
+    expect(isValidCellMarks({})).toBe(false);
+    expect(isValidCellMarks(42)).toBe(false);
+  });
+
+  it("rejects arrays containing malformed entries", () => {
+    expect(isValidCellMarks([null], schema)).toBe(false);
+    expect(isValidCellMarks(["strong"], schema)).toBe(false);
+    expect(isValidCellMarks([{}], schema)).toBe(false);
+    expect(isValidCellMarks([{ type: 123 }], schema)).toBe(false);
+    expect(isValidCellMarks([{ type: "strong", attrs: "oops" }], schema)).toBe(
+      false
+    );
+  });
+
+  it("accepts registered marks without attrs", () => {
+    expect(isValidCellMarks([{ type: "strong" }], schema)).toBe(true);
+    expect(isValidCellMarks([{ type: "em" }], schema)).toBe(true);
+  });
+});

shared/editor/lib/table.ts

@@ -1,6 +1,7 @@
-import type { Attrs, Node } from "prosemirror-model";
+import type { Attrs, Node, Schema } from "prosemirror-model";
 import type { MutableAttrs } from "prosemirror-tables";
 import { isBrowser } from "../../utils/browser";
+import { validateColorHex } from "../../utils/color";
 import type { TableLayout, NodeAttrMark } from "../types";
 import { readableColor } from "polished";
 
@@ -16,6 +17,65 @@ export interface CellAttrs {
   marks?: NodeAttrMark[];
 }
 
+const ALLOWED_ALIGNMENTS = new Set(["left", "center", "right"]);
+
+/**
+ * Validates an alignment attribute value.
+ *
+ * @param value The value to validate.
+ * @returns true if the value is a safe alignment or null.
+ */
+export const isValidCellAlignment = (
+  value: unknown
+): value is "left" | "center" | "right" | null =>
+  value === null ||
+  (typeof value === "string" && ALLOWED_ALIGNMENTS.has(value));
+
+/**
+ * Validates a table cell's `marks` attribute against the given schema. Checks
+ * that the value is an array of well-formed mark objects whose type exists in
+ * the schema, and — for `background` marks — that the color is a valid hex
+ * value. `null` and `undefined` are both considered valid (the attribute is
+ * optional).
+ *
+ * @param value The value to validate.
+ * @param schema The editor schema, used to check mark types are registered.
+ *               Optional — when absent, mark-type registration is not checked.
+ * @returns true if the value is a valid marks array, null, or undefined.
+ */
+export const isValidCellMarks = (
+  value: unknown,
+  schema?: Schema
+): value is NodeAttrMark[] | null | undefined => {
+  if (value === undefined || value === null) {
+    return true;
+  }
+  if (!Array.isArray(value)) {
+    return false;
+  }
+  const marks = schema?.marks;
+  return value.every((mark) => {
+    if (!mark || typeof mark !== "object") {
+      return false;
+    }
+    const type = (mark as NodeAttrMark).type;
+    if (typeof type !== "string") {
+      return false;
+    }
+    if (marks && !Object.prototype.hasOwnProperty.call(marks, type)) {
+      return false;
+    }
+    const attrs = (mark as NodeAttrMark).attrs;
+    if (attrs !== undefined && (typeof attrs !== "object" || attrs === null)) {
+      return false;
+    }
+    if (type === "background") {
+      return typeof attrs?.color === "string" && validateColorHex(attrs.color);
+    }
+    return true;
+  });
+};
+
 /**
  * Helper to get cell attributes from a DOM node, used when pasting table content.
  *
@@ -34,6 +94,8 @@ export function getCellAttrs(dom: HTMLElement | string): Attrs {
       : null;
   const colspan = Number(dom.getAttribute("colspan") || 1);
 
+  const bgColor = dom.getAttribute("data-bgcolor");
+
   return {
     colspan,
     rowspan: Number(dom.getAttribute("rowspan") || 1),
@@ -44,16 +106,17 @@ export function getCellAttrs(dom: HTMLElement | string): Attrs {
         : dom.style.textAlign === "right"
           ? "right"
           : null,
-    marks: dom.getAttribute("data-bgcolor")
-      ? [
-          {
-            type: "background",
-            attrs: {
-              color: dom.getAttribute("data-bgcolor"),
+    marks:
+      bgColor && validateColorHex(bgColor)
+        ? [
+            {
+              type: "background",
+              attrs: {
+                color: bgColor,
+              },
             },
-          },
-        ]
-      : undefined,
+          ]
+        : undefined,
   } satisfies CellAttrs;
 }
 
@@ -71,7 +134,7 @@ export function setCellAttrs(node: Node): Attrs {
   if (node.attrs.rowspan !== 1) {
     attrs.rowspan = node.attrs.rowspan;
   }
-  if (node.attrs.alignment) {
+  if (isValidCellAlignment(node.attrs.alignment) && node.attrs.alignment) {
     attrs.style = `text-align: ${node.attrs.alignment};`;
   }
   if (node.attrs.colwidth) {
@@ -82,15 +145,19 @@ export function setCellAttrs(node: Node): Attrs {
         (attrs.style ?? "") + `min-width: ${Number(node.attrs.colwidth[0])}px;`;
     }
   }
-  if (node.attrs.marks) {
+  if (Array.isArray(node.attrs.marks)) {
     const backgroundMark = node.attrs.marks.find(
-      (mark: NodeAttrMark) => mark.type === "background"
+      (mark: NodeAttrMark) =>
+        mark?.type === "background" &&
+        typeof mark.attrs?.color === "string" &&
+        validateColorHex(mark.attrs.color)
     );
     if (backgroundMark) {
-      attrs["data-bgcolor"] = backgroundMark.attrs.color;
+      const color = backgroundMark.attrs!.color as string;
+      attrs["data-bgcolor"] = color;
       attrs.style =
         (attrs.style ?? "") +
-        `--cell-bg-color: ${backgroundMark.attrs.color}; --cell-text-color: ${readableColor(backgroundMark.attrs.color)};`;
+        `--cell-bg-color: ${color}; --cell-text-color: ${readableColor(color)};`;
     }
   }
 

shared/editor/nodes/TableCell.ts

@@ -8,7 +8,12 @@ import type { EditorState } from "prosemirror-state";
 import { Plugin, PluginKey } from "prosemirror-state";
 import { Decoration, DecorationSet } from "prosemirror-view";
 import { TableMap } from "prosemirror-tables";
-import { getCellAttrs, setCellAttrs } from "../lib/table";
+import {
+  getCellAttrs,
+  isValidCellAlignment,
+  isValidCellMarks,
+  setCellAttrs,
+} from "../lib/table";
 import Node from "./Node";
 import { presetColors, rgbaToHex } from "@shared/utils/color";
 import { parseToRgb, transparentize } from "polished";
@@ -53,10 +58,12 @@ export default class TableCell extends Node {
       attrs: {
         colspan: { default: 1 },
         rowspan: { default: 1 },
-        alignment: { default: null },
+        alignment: { default: null, validate: isValidCellAlignment },
         colwidth: { default: null },
         marks: {
           default: undefined,
+          validate: (value: unknown) =>
+            isValidCellMarks(value, this.editor?.schema),
         },
       },
     };
@@ -69,7 +76,9 @@ export default class TableCell extends Node {
   parseMarkdown() {
     return {
       block: "td",
-      getAttrs: (tok: Token) => ({ alignment: tok.info }),
+      getAttrs: (tok: Token) => ({
+        alignment: isValidCellAlignment(tok.info) ? tok.info : null,
+      }),
     };
   }
 

shared/editor/nodes/TableHeader.ts

@@ -6,7 +6,12 @@ import type { EditorView } from "prosemirror-view";
 import { DecorationSet, Decoration } from "prosemirror-view";
 import { isInTable, moveTableColumn, TableMap } from "prosemirror-tables";
 import { addColumnBefore, selectColumn } from "../commands/table";
-import { getCellAttrs, setCellAttrs } from "../lib/table";
+import {
+  getCellAttrs,
+  isValidCellAlignment,
+  isValidCellMarks,
+  setCellAttrs,
+} from "../lib/table";
 import {
   getCellsInColumn,
   getCellsInRow,
@@ -208,10 +213,12 @@ export default class TableHeader extends Node {
       attrs: {
         colspan: { default: 1 },
         rowspan: { default: 1 },
-        alignment: { default: null },
+        alignment: { default: null, validate: isValidCellAlignment },
         colwidth: { default: null },
         marks: {
           default: undefined,
+          validate: (value: unknown) =>
+            isValidCellMarks(value, this.editor?.schema),
         },
       },
     };
@@ -224,7 +231,9 @@ export default class TableHeader extends Node {
   parseMarkdown() {
     return {
       block: "th",
-      getAttrs: (tok: Token) => ({ alignment: tok.info }),
+      getAttrs: (tok: Token) => ({
+        alignment: isValidCellAlignment(tok.info) ? tok.info : null,
+      }),
     };
   }
 

shared/utils/color.test.ts

@@ -0,0 +1,56 @@
+import { validateColorHex } from "./color";
+
+describe("validateColorHex", () => {
+  it("accepts 3-digit hex", () => {
+    expect(validateColorHex("#fff")).toBe(true);
+    expect(validateColorHex("#ABC")).toBe(true);
+    expect(validateColorHex("#000")).toBe(true);
+  });
+
+  it("accepts 4-digit hex with alpha", () => {
+    expect(validateColorHex("#fff8")).toBe(true);
+    expect(validateColorHex("#ABCD")).toBe(true);
+  });
+
+  it("accepts 6-digit hex", () => {
+    expect(validateColorHex("#FDEA9B")).toBe(true);
+    expect(validateColorHex("#ffffff")).toBe(true);
+  });
+
+  it("accepts 8-digit hex with alpha", () => {
+    expect(validateColorHex("#FDEA9BB3")).toBe(true);
+    expect(validateColorHex("#00000080")).toBe(true);
+  });
+
+  it("rejects strings missing the leading #", () => {
+    expect(validateColorHex("ffffff")).toBe(false);
+    expect(validateColorHex("fff")).toBe(false);
+  });
+
+  it("rejects hex strings of unsupported lengths", () => {
+    expect(validateColorHex("#")).toBe(false);
+    expect(validateColorHex("#f")).toBe(false);
+    expect(validateColorHex("#ff")).toBe(false);
+    expect(validateColorHex("#fffff")).toBe(false);
+    expect(validateColorHex("#fffffff")).toBe(false);
+    expect(validateColorHex("#fffffffff")).toBe(false);
+  });
+
+  it("rejects strings containing non-hex characters", () => {
+    expect(validateColorHex("#fffz")).toBe(false);
+    expect(validateColorHex("#xyz")).toBe(false);
+    expect(validateColorHex("#GGG")).toBe(false);
+  });
+
+  it("rejects color names and CSS values", () => {
+    expect(validateColorHex("red")).toBe(false);
+    expect(validateColorHex("rgb(0, 0, 0)")).toBe(false);
+  });
+
+  it("rejects strings with CSS injection payloads", () => {
+    expect(
+      validateColorHex("#fff; background-image: url(https://evil.example)")
+    ).toBe(false);
+    expect(validateColorHex("#fff;")).toBe(false);
+  });
+});

shared/utils/color.ts

@@ -21,7 +21,7 @@ export const palette = [
 ];
 
 export const validateColorHex = (color: string) =>
-  /(^#[0-9A-F]{6}$)|(^#[0-9A-F]{3}$)/i.test(color);
+  /^#(?:[0-9A-F]{3,4}|[0-9A-F]{6}|[0-9A-F]{8})$/i.test(color);
 
 export const stringToColor = (input: string) => {
   const inputAsNumber = parseInt(md5(input).toString(), 16);