diff --git a/package.json b/package.json index 5816786e3..77e110c3f 100644 --- a/package.json +++ b/package.json @@ -20,6 +20,7 @@ }, "dependencies": { "@dwelle/tunnel-rat": "0.1.1", + "@braintree/sanitize-url": "6.0.2", "@sentry/browser": "6.2.5", "@sentry/integrations": "6.2.5", "@testing-library/jest-dom": "5.16.2", diff --git a/src/components/App.tsx b/src/components/App.tsx index 413a130d8..30c0408b7 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -279,13 +279,12 @@ import { } 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 { Fonts } from "../scene/Fonts"; import { actionPaste } from "../actions/actionClipboard"; @@ -2947,12 +2946,19 @@ class App extends React.Component { 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"; @@ -2960,7 +2966,7 @@ class App extends React.Component { // 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 fcf5fa132..194e528b9 100644 --- a/src/data/restore.ts +++ b/src/data/restore.ts @@ -40,6 +40,7 @@ import { getDefaultLineHeight, measureBaseline, } from "../element/textElement"; +import { normalizeLink } from "./url"; type RestoredAppState = Omit< AppState, @@ -139,7 +140,7 @@ const restoreElementWithProperties = < ? 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, }; diff --git a/src/data/url.test.tsx b/src/data/url.test.tsx new file mode 100644 index 000000000..36bed0a38 --- /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("")).toBe(""); + }); +}); diff --git a/src/data/url.ts b/src/data/url.ts new file mode 100644 index 000000000..d34320a5d --- /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 43cbf0da5..14e77877d 100644 --- a/src/element/Hyperlink.tsx +++ b/src/element/Hyperlink.tsx @@ -29,6 +29,7 @@ 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"; @@ -166,7 +167,7 @@ export const Hyperlink = ({ /> ) : ( { - 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) => { diff --git a/src/packages/excalidraw/index.tsx b/src/packages/excalidraw/index.tsx index 4f768c6d5..0c685262f 100644 --- a/src/packages/excalidraw/index.tsx +++ b/src/packages/excalidraw/index.tsx @@ -245,3 +245,4 @@ export { MainMenu }; export { useDevice } from "../../components/App"; export { WelcomeScreen }; export { LiveCollaborationTrigger }; +export { normalizeLink } from "../../data/url"; diff --git a/src/renderer/renderElement.ts b/src/renderer/renderElement.ts index f85c83a6b..89c69972f 100644 --- a/src/renderer/renderElement.ts +++ b/src/renderer/renderElement.ts @@ -48,6 +48,7 @@ import { getMaxContainerWidth, } from "../element/textElement"; import { LinearElementEditor } from "../element/linearElementEditor"; +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 @@ -1140,7 +1141,7 @@ export const renderElementToSvg = ( // 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; } diff --git a/yarn.lock b/yarn.lock index 89d153909..f0f6181c1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1326,6 +1326,11 @@ 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"