CVE-2023-26140: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Versions of the package @excalidraw/excalidraw from 0.0.0 are vulnerable to Cross-site Scripting (XSS) via embedded links in whiteboard objects due to improper input sanitization.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2023-08-16T05:15Z
Repository: https://github.com/excalidraw/excalidraw
Language: Other
Files changed: package.json, src/components/App.tsx, src/data/restore.ts, src/data/url.test.tsx, src/data/url.ts, src/element/Hyperlink.tsx, src/packages/excalidraw/CHANGELOG.md, src/packages/excalidraw/index.tsx, src/renderer/renderElement.ts, yarn.lock
Commit date: 2023-06-29 12:14:42 +0200
Commit message: fix: stronger enforcement of normalizeLink (#6728)

Co-authored-by: dwelle <luzar.david@gmail.com>

Diff: diff --git a/package.json b/package.json
index 584bdcbc..d1198fa2 100644
--- a/package.json
+++ b/package.json
@@ -17,10 +17,11 @@
       "last 1 firefox version",
       "last 1 safari version"
     ]
   },
   "dependencies": {
+    "@braintree/sanitize-url": "6.0.2",
     "@excalidraw/random-username": "1.0.0",
     "@radix-ui/react-popover": "1.0.3",
     "@radix-ui/react-tabs": "1.0.2",
     "@sentry/browser": "6.2.5",
     "@sentry/integrations": "6.2.5",
diff --git a/src/components/App.tsx b/src/components/App.tsx
index 99be4b0d..2479731e 100644
--- a/src/components/App.tsx
+++ b/src/components/App.tsx
@@ -289,17 +289,16 @@ import {
   isMeasureTextSupported,
   isValidTextContainer,
 } from "../element/textElement";
 import { isHittingElementNotConsideringBoundingBox } from "../element/collision";
 import {
-  normalizeLink,
   showHyperlinkTooltip,
   hideHyperlinkToolip,
   Hyperlink,
   isPointHittingLinkIcon,
-  isLocalLink,
 } from "../element/Hyperlink";
+import { isLocalLink, normalizeLink } from "../data/url";
 import { shouldShowBoundingBox } from "../element/transformHandles";
 import { actionUnlockAllElements } from "../actions/actionElementLock";
 import { Fonts } from "../scene/Fonts";
 import {
   getFrameElements,
@@ -3350,24 +3349,31 @@ class App extends React.Component<AppProps, AppState> {
       this.state,
       [lastPointerUpCoords.x, lastPointerUpCoords.y],
       this.device.isMobile,
     );
     if (lastPointerDownHittingLinkIcon && lastPointerUpHittingLinkIcon) {
-      const url = this.hitLinkElement.link;
+      let url = this.hitLinkElement.link;
       if (url) {
+        url = normalizeLink(url);
         let customEvent;
         if (this.props.onLinkOpen) {
           customEvent = wrapEvent(EVENT.EXCALIDRAW_LINK, event.nativeEvent);
-          this.props.onLinkOpen(this.hitLinkElement, customEvent);
+          this.props.onLinkOpen(
+            {
+              ...this.hitLinkElement,
+              link: url,
+            },
+            customEvent,
+          );
         }
         if (!customEvent?.defaultPrevented) {
           const target = isLocalLink(url) ? "_self" : "_blank";
           const newWindow = window.open(undefined, target);
           // https://mathiasbynens.github.io/rel-noopener/
           if (newWindow) {
             newWindow.opener = null;
-            newWindow.location = normalizeLink(url);
+            newWindow.location = url;
           }
         }
       }
     }
   };
diff --git a/src/data/restore.ts b/src/data/restore.ts
index 57a0265f..5f2adc00 100644
--- a/src/data/restore.ts
+++ b/src/data/restore.ts
@@ -39,10 +39,11 @@ import {
   detectLineHeight,
   getDefaultLineHeight,
   measureBaseline,
 } from "../element/textElement";
 import { COLOR_PALETTE } from "../colors";
+import { normalizeLink } from "./url";
 
 type RestoredAppState = Omit<
   AppState,
   "offsetTop" | "offsetLeft" | "width" | "height"
 >;
@@ -140,11 +141,11 @@ const restoreElementWithProperties = <
       : null,
     boundElements: element.boundElementIds
       ? element.boundElementIds.map((id) => ({ type: "arrow", id }))
       : element.boundElements ?? [],
     updated: element.updated ?? getUpdatedTimestamp(),
-    link: element.link ?? null,
+    link: element.link ? normalizeLink(element.link) : null,
     locked: element.locked ?? false,
   };
 
   if ("customData" in element) {
     base.customData = element.customData;
diff --git a/src/data/url.test.tsx b/src/data/url.test.tsx
new file mode 100644
index 00000000..36bed0a3
--- /dev/null
+++ b/src/data/url.test.tsx
@@ -0,0 +1,30 @@
+import { normalizeLink } from "./url";
+
+describe("normalizeLink", () => {
+  // NOTE not an extensive XSS test suite, just to check if we're not
+  // regressing in sanitization
+  it("should sanitize links", () => {
+    expect(
+      // eslint-disable-next-line no-script-url
+      normalizeLink(`javascript://%0aalert(document.domain)`).startsWith(
+        // eslint-disable-next-line no-script-url
+        `javascript:`,
+      ),
+    ).toBe(false);
+    expect(normalizeLink("ola")).toBe("ola");
+    expect(normalizeLink(" ola")).toBe("ola");
+
+    expect(normalizeLink("https://www.excalidraw.com")).toBe(
+      "https://www.excalidraw.com",
+    );
+    expect(normalizeLink("www.excalidraw.com")).toBe("www.excalidraw.com");
+    expect(normalizeLink("/ola")).toBe("/ola");
+    expect(normalizeLink("http://test")).toBe("http://test");
+    expect(normalizeLink("ftp://test")).toBe("ftp://test");
+    expect(normalizeLink("file://")).toBe("file://");
+    expect(normalizeLink("file://")).toBe("file://");
+    expect(normalizeLink("[test](https://test)")).toBe("[test](https://test)");
+    expect(normalizeLink("[[test]]")).toBe("[[test]]");
+    expect(normalizeLink("<test>")).toBe("<test>");
+  });
+});
diff --git a/src/data/url.ts b/src/data/url.ts
new file mode 100644
index 00000000..d34320a5
--- /dev/null
+++ b/src/data/url.ts
@@ -0,0 +1,9 @@
+import { sanitizeUrl } from "@braintree/sanitize-url";
+
+export const normalizeLink = (link: string) => {
+  return sanitizeUrl(link);
+};
+
+export const isLocalLink = (link: string | null) => {
+  return !!(link?.includes(location.origin) || link?.startsWith("/"));
+};
diff --git a/src/element/Hyperlink.tsx b/src/element/Hyperlink.tsx
index 6bf3b17e..cf741ce0 100644
--- a/src/element/Hyperlink.tsx
+++ b/src/element/Hyperlink.tsx
@@ -27,10 +27,11 @@ import { EVENT, HYPERLINK_TOOLTIP_DELAY, MIME_TYPES } from "../constants";
 import { Bounds } from "./bounds";
 import { getTooltipDiv, updateTooltipPosition } from "../components/Tooltip";
 import { getSelectedElements } from "../scene";
 import { isPointHittingElementBoundingBox } from "./collision";
 import { getElementAbsoluteCoords } from "./";
+import { isLocalLink, normalizeLink } from "../data/url";
 
 import "./Hyperlink.scss";
 import { trackEvent } from "../analytics";
 import { useExcalidrawAppState } from "../components/App";
 
@@ -164,22 +165,28 @@ export const Hyperlink = ({
             }
           }}
         />
       ) : (
         <a
-          href={element.link || ""}
+          href={normalizeLink(element.link || "")}
           className={clsx("excalidraw-hyperlinkContainer-link", {
             "d-none": isEditing,
           })}
           target={isLocalLink(element.link) ? "_self" : "_blank"}
           onClick={(event) => {
             if (element.link && onLinkOpen) {
               const customEvent = wrapEvent(
                 EVENT.EXCALIDRAW_LINK,
                 event.nativeEvent,
               );
-              onLinkOpen(element, customEvent);
+              onLinkOpen(
+                {
+                  ...element,
+                  link: normalizeLink(element.link),
+                },
+                customEvent,
+              );
               if (customEvent.defaultPrevented) {
                 event.preventDefault();
               }
             }
           }}
@@ -229,25 +236,10 @@ const getCoordsForPopover = (
   const x = viewportX - appState.offsetLeft - CONTAINER_WIDTH / 2;
   const y = viewportY - appState.offsetTop - SPACE_BOTTOM;
   return { x, y };
 };
 
-export const normalizeLink = (link: string) => {
-  link = link.trim();
-  if (link) {
-    // prefix with protocol if not fully-qualified
-    if (!link.includes("://") && !/^[[\\/]/.test(link)) {
-      link = `https://${link}`;
-    }
-  }
-  return link;
-};
-
-export const isLocalLink = (link: string | null) => {
-  return !!(link?.includes(location.origin) || link?.startsWith("/"));
-};
-
 export const actionLink = register({
   name: "hyperlink",
   perform: (elements, appState) => {
     if (appState.showHyperlinkPopup === "editor") {
       return false;
diff --git a/src/packages/excalidraw/CHANGELOG.md b/src/packages/excalidraw/CHANGELOG.md
index 30292159..1cd27d95 100644
--- a/src/packages/excalidraw/CHANGELOG.md
+++ b/src/packages/excalidraw/CHANGELOG.md
@@ -13,10 +13,11 @@ Please add the latest change on the top under the correct section.
 
 ## Unreleased
 
 ### Features
 
+- Properly sanitize element `link` urls. [#6728](https://github.com/excalidraw/excalidraw/pull/6728).
 - Sidebar component now supports tabs — for more detailed description of new behavior and breaking changes, see the linked PR. [#6213](https://github.com/excalidraw/excalidraw/pull/6213)
 - Exposed `DefaultSidebar` component to allow modifying the default sidebar, such as adding custom tabs to it. [#6213](https://github.com/excalidraw/excalidraw/pull/6213)
 
   #### BREAKING CHANGES
 
diff --git a/src/packages/excalidraw/index.tsx b/src/packages/excalidraw/index.tsx
index 22f79dd3..8ee2956b 100644
--- a/src/packages/excalidraw/index.tsx
+++ b/src/packages/excalidraw/index.tsx
@@ -245,5 +245,7 @@ export { MainMenu };
 export { useDevice } from "../../components/App";
 export { WelcomeScreen };
 export { LiveCollaborationTrigger };
 
 export { DefaultSidebar } from "../../components/DefaultSidebar";
+
+export { normalizeLink } from "../../data/url";
diff --git a/src/renderer/renderElement.ts b/src/renderer/renderElement.ts
index ed6ccb69..0efe5df9 100644
--- a/src/renderer/renderElement.ts
+++ b/src/renderer/renderElement.ts
@@ -48,10 +48,11 @@ import {
   getBoundTextMaxHeight,
   getBoundTextMaxWidth,
 } from "../element/textElement";
 import { LinearElementEditor } from "../element/linearElementEditor";
 import { getContainingFrame } from "../frame";
+import { normalizeLink } from "../data/url";
 
 // using a stronger invert (100% vs our regular 93%) and saturate
 // as a temp hack to make images in dark theme look closer to original
 // color scheme (it's still not quite there and the colors look slightly
 // desatured, alas...)
@@ -1201,11 +1202,11 @@ export const renderElementToSvg = (
   let root = svgRoot;
 
   // if the element has a link, create an anchor tag and make that the new root
   if (element.link) {
     const anchorTag = svgRoot.ownerDocument!.createElementNS(SVG_NS, "a");
-    anchorTag.setAttribute("href", element.link);
+    anchorTag.setAttribute("href", normalizeLink(element.link));
     root.appendChild(anchorTag);
     root = anchorTag;
   }
 
   const opacity =
diff --git a/yarn.lock b/yarn.lock
index 2e3c1ea8..2e8bdda5 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1084,10 +1084,15 @@
 "@bcoe/v8-coverage@^0.2.3":
   version "0.2.3"
   resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39"
   integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==
 
+"@braintree/sanitize-url@6.0.2":
+  version "6.0.2"
+  resolved "https://registry.yarnpkg.com/@braintree/sanitize-url/-/sanitize-url-6.0.2.tgz#6110f918d273fe2af8ea1c4398a88774bb9fc12f"
+  integrity sha512-Tbsj02wXCbqGmzdnXNk0SOF19ChhRU70BsroIi4Pm6Ehp56in6vch94mfbdQ17DozxkL3BAVjbZ4Qc1a0HFRAg==
+
 "@csstools/normalize.css@*":
   version "12.0.0"
   resolved "https://registry.yarnpkg.com/@csstools/normalize.css/-/normalize.css-12.0.0.tgz#a9583a75c3f150667771f30b60d9f059473e62c4"
   integrity sha512-M0qqxAcwCsIVfpFQSlGN5XjXWu8l5JDZN+fPt1LeW5SZexQTgnaEvgXAY+CeygRw0EeppWHi12JxESWiWrB0Sg==
 

Vulnerable code:   normalizeLink,
  isLocalLink,
      const url = this.hitLinkElement.link;
          this.props.onLinkOpen(this.hitLinkElement, customEvent);
            newWindow.location = normalizeLink(url);
    link: element.link ?? null,
          href={element.link || ""}
              onLinkOpen(element, customEvent);
export const normalizeLink = (link: string) => {
  link = link.trim();
  if (link) {
    // prefix with protocol if not fully-qualified
    if (!link.includes("://") && !/^[[\\/]/.test(link)) {
      link = `https://${link}`;
    }
  }
  return link;
};

export const isLocalLink = (link: string | null) => {
  return !!(link?.includes(location.origin) || link?.startsWith("/"));
};

    anchorTag.setAttribute("href", element.link);
Fixed code:     "@braintree/sanitize-url": "6.0.2",
import { isLocalLink, normalizeLink } from "../data/url";
      let url = this.hitLinkElement.link;
        url = normalizeLink(url);
          this.props.onLinkOpen(
            {
              ...this.hitLinkElement,
              link: url,
            },
            customEvent,
          );
            newWindow.location = url;
import { normalizeLink } from "./url";
    link: element.link ? normalizeLink(element.link) : null,
import { normalizeLink } from "./url";

describe("normalizeLink", () => {
  // NOTE not an extensive XSS test suite, just to check if we're not
  // regressing in sanitization
  it("should sanitize links", () => {
    expect(
      // eslint-disable-next-line no-script-url
      normalizeLink(`javascript://%0aalert(document.domain)`).startsWith(
        // eslint-disable-next-line no-script-url
        `javascript:`,
      ),
    ).toBe(false);
    expect(normalizeLink("ola")).toBe("ola");
    expect(normalizeLink(" ola")).toBe("ola");

    expect(normalizeLink("https://www.excalidraw.com")).toBe(
      "https://www.excalidraw.com",
    );
    expect(normalizeLink("www.excalidraw.com")).toBe("www.excalidraw.com");
    expect(normalizeLink("/ola")).toBe("/ola");
    expect(normalizeLink("http://test")).toBe("http://test");
    expect(normalizeLink("ftp://test")).toBe("ftp://test");
    expect(normalizeLink("file://")).toBe("file://");
    expect(normalizeLink("file://")).toBe("file://");
    expect(normalizeLink("[test](https://test)")).toBe("[test](https://test)");
    expect(normalizeLink("[[test]]")).toBe("[[test]]");
    expect(normalizeLink("<test>")).toBe("<test>");
  });
});
import { sanitizeUrl } from "@braintree/sanitize-url";

export const normalizeLink = (link: string) => {
  return sanitizeUrl(link);
};

export const isLocalLink = (link: string | null) => {
  return !!(link?.includes(location.origin) || link?.startsWith("/"));
};
import { isLocalLink, normalizeLink } from "../data/url";
          href={normalizeLink(element.link || "")}
              onLinkOpen(
                {
                  ...element,
                  link: normalizeLink(element.link),
                },
                customEvent,
              );
- Properly sanitize element `link` urls. [#6728](https://github.com/excalidraw/excalidraw/pull/6728).

export { normalizeLink } from "../../data/url";
import { normalizeLink } from "../data/url";
    anchorTag.setAttribute("href", normalizeLink(element.link));
"@braintree/sanitize-url@6.0.2":
  version "6.0.2"
  resolved "https://registry.yarnpkg.com/@braintree/sanitize-url/-/sanitize-url-6.0.2.tgz#6110f918d273fe2af8ea1c4398a88774bb9fc12f"
  integrity sha512-Tbsj02wXCbqGmzdnXNk0SOF19ChhRU70BsroIi4Pm6Ehp56in6vch94mfbdQ17DozxkL3BAVjbZ4Qc1a0HFRAg==