From b63689c23095d7a5fcea35a8d1e892f3ddacc1d0 Mon Sep 17 00:00:00 2001
From: David Luzar <5153846+dwelle@users.noreply.github.com>
Date: Sun, 5 Jan 2025 21:45:04 +0100
Subject: [PATCH] feat: make HTML attribute sanitization stricter (#8977)

* feat: make HTML attribute sanitization stricter

* fix double escape
---
 packages/excalidraw/data/url.test.tsx     |  3 ++-
 packages/excalidraw/data/url.ts           |  5 +----
 packages/excalidraw/element/embeddable.ts |  7 +++++--
 packages/excalidraw/tests/utils.test.ts   | 16 +++++++++++-----
 packages/excalidraw/utils.ts              | 13 +++++++++++++
 5 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/packages/excalidraw/data/url.test.tsx b/packages/excalidraw/data/url.test.tsx
index 36bed0a38a..e0e07797dd 100644
--- a/packages/excalidraw/data/url.test.tsx
+++ b/packages/excalidraw/data/url.test.tsx
@@ -25,6 +25,7 @@ describe("normalizeLink", () => {
     expect(normalizeLink("file://")).toBe("file://");
     expect(normalizeLink("[test](https://test)")).toBe("[test](https://test)");
     expect(normalizeLink("[[test]]")).toBe("[[test]]");
-    expect(normalizeLink("<test>")).toBe("<test>");
+    expect(normalizeLink("<test>")).toBe("&lt;test&gt;");
+    expect(normalizeLink("test&")).toBe("test&amp;");
   });
 });
diff --git a/packages/excalidraw/data/url.ts b/packages/excalidraw/data/url.ts
index dae5760680..e0c7323db1 100644
--- a/packages/excalidraw/data/url.ts
+++ b/packages/excalidraw/data/url.ts
@@ -1,8 +1,5 @@
 import { sanitizeUrl } from "@braintree/sanitize-url";
-
-export const sanitizeHTMLAttribute = (html: string) => {
-  return html.replace(/"/g, "&quot;");
-};
+import { sanitizeHTMLAttribute } from "../utils";
 
 export const normalizeLink = (link: string) => {
   link = link.trim();
diff --git a/packages/excalidraw/element/embeddable.ts b/packages/excalidraw/element/embeddable.ts
index 9bc4a139b1..0948d4b520 100644
--- a/packages/excalidraw/element/embeddable.ts
+++ b/packages/excalidraw/element/embeddable.ts
@@ -1,7 +1,11 @@
 import { register } from "../actions/register";
 import { FONT_FAMILY, VERTICAL_ALIGN } from "../constants";
 import type { ExcalidrawProps } from "../types";
-import { getFontString, updateActiveTool } from "../utils";
+import {
+  getFontString,
+  sanitizeHTMLAttribute,
+  updateActiveTool,
+} from "../utils";
 import { setCursorForShape } from "../cursor";
 import { newTextElement } from "./newElement";
 import { wrapText } from "./textWrapping";
@@ -11,7 +15,6 @@ import type {
   ExcalidrawIframeLikeElement,
   IframeData,
 } from "./types";
-import { sanitizeHTMLAttribute } from "../data/url";
 import type { MarkRequired } from "../utility-types";
 import { StoreAction } from "../store";
 
diff --git a/packages/excalidraw/tests/utils.test.ts b/packages/excalidraw/tests/utils.test.ts
index 24371836a7..3663973c38 100644
--- a/packages/excalidraw/tests/utils.test.ts
+++ b/packages/excalidraw/tests/utils.test.ts
@@ -1,13 +1,19 @@
-import * as utils from "../utils";
+import { isTransparent, sanitizeHTMLAttribute } from "../utils";
 
 describe("Test isTransparent", () => {
   it("should return true when color is rgb transparent", () => {
-    expect(utils.isTransparent("#ff00")).toEqual(true);
-    expect(utils.isTransparent("#fff00000")).toEqual(true);
-    expect(utils.isTransparent("transparent")).toEqual(true);
+    expect(isTransparent("#ff00")).toEqual(true);
+    expect(isTransparent("#fff00000")).toEqual(true);
+    expect(isTransparent("transparent")).toEqual(true);
   });
 
   it("should return false when color is not transparent", () => {
-    expect(utils.isTransparent("#ced4da")).toEqual(false);
+    expect(isTransparent("#ced4da")).toEqual(false);
+  });
+});
+
+describe("sanitizeHTMLAttribute()", () => {
+  it("should escape HTML attribute special characters & not double escape", () => {
+    expect(sanitizeHTMLAttribute(`&"'><`)).toBe("&amp;&quot;&#39;&gt;&lt;");
   });
 });
diff --git a/packages/excalidraw/utils.ts b/packages/excalidraw/utils.ts
index 8176b1a747..1c1cdc1494 100644
--- a/packages/excalidraw/utils.ts
+++ b/packages/excalidraw/utils.ts
@@ -1225,3 +1225,16 @@ export class PromisePool<T> {
     });
   }
 }
+
+export const sanitizeHTMLAttribute = (html: string) => {
+  return (
+    html
+      // note, if we're not doing stupid things, escaping " is enough,
+      // but we might end up doing stupid things
+      .replace(/&/g, "&amp;")
+      .replace(/"/g, "&quot;")
+      .replace(/'/g, "&#39;")
+      .replace(/>/g, "&gt;")
+      .replace(/</g, "&lt;")
+  );
+};