Improved sanitization of href's in editor

app/editor/components/LinkEditor.tsx

@@ -11,8 +11,7 @@ import { setTextSelection } from "prosemirror-utils";
 import { EditorView } from "prosemirror-view";
 import * as React from "react";
 import styled from "styled-components";
-import isUrl from "@shared/editor/lib/isUrl";
-import { isInternalUrl } from "@shared/utils/urls";
+import { isInternalUrl, sanitizeHref } from "@shared/utils/urls";
 import Flex from "~/components/Flex";
 import { Dictionary } from "~/hooks/useDictionary";
 import { ToastOptions } from "~/types";
@@ -114,17 +113,7 @@ class LinkEditor extends React.Component<Props, State> {
 
     this.discardInputValue = true;
     const { from, to } = this.props;
-
-    // Make sure a protocol is added to the beginning of the input if it's
-    // likely an absolute URL that was entered without one.
-    if (
-      !isUrl(href) &&
-      !href.startsWith("/") &&
-      !href.startsWith("#") &&
-      !href.startsWith("mailto:")
-    ) {
-      href = `https://${href}`;
-    }
+    href = sanitizeHref(href);
 
     this.props.onSelectLink({ href, title, from, to });
   };

app/hooks/useDictionary.ts

@@ -53,6 +53,7 @@ export default function useDictionary() {
       noResults: t("No results"),
       openLink: t("Open link"),
       goToLink: t("Go to link"),
+      openLinkError: t("Sorry, that type of link is not supported"),
       orderedList: t("Ordered list"),
       pageBreak: t("Page break"),
       pasteLink: `${t("Paste a link")}…`,

shared/editor/lib/isUrl.ts

@@ -1,12 +0,0 @@
-export default function isUrl(text: string) {
-  if (text.match(/\n/)) {
-    return false;
-  }
-
-  try {
-    const url = new URL(text);
-    return url.hostname !== "";
-  } catch (err) {
-    return false;
-  }
-}

shared/editor/marks/Link.tsx

@@ -13,7 +13,7 @@ import { EditorState, Plugin } from "prosemirror-state";
 import { Decoration, DecorationSet } from "prosemirror-view";
 import * as React from "react";
 import ReactDOM from "react-dom";
-import { isExternalUrl } from "../../utils/urls";
+import { isExternalUrl, sanitizeHref } from "../../utils/urls";
 import findLinkNodes from "../queries/findLinkNodes";
 import { EventType, Dispatch } from "../types";
 import Mark from "./Mark";
@@ -80,6 +80,7 @@ export default class Link extends Mark {
         "a",
         {
           ...node.attrs,
+          href: sanitizeHref(node.attrs.href),
           rel: "noopener noreferrer nofollow",
         },
         0,
@@ -196,18 +197,25 @@ export default class Link extends Mark {
                   ? event.target.parentNode.href
                   : "");
 
-              const isHashtag = href.startsWith("#");
-              if (isHashtag && this.options.onClickHashtag) {
-                event.stopPropagation();
-                event.preventDefault();
-                this.options.onClickHashtag(href, event);
+              try {
+                const isHashtag = href.startsWith("#");
+                if (isHashtag && this.options.onClickHashtag) {
+                  event.stopPropagation();
+                  event.preventDefault();
+                  this.options.onClickHashtag(href, event);
+                }
+
+                if (this.options.onClickLink) {
+                  event.stopPropagation();
+                  event.preventDefault();
+                  this.options.onClickLink(href, event);
+                }
+              } catch (err) {
+                this.editor.props.onShowToast(
+                  this.options.dictionary.openLinkError
+                );
               }
 
-              if (this.options.onClickLink) {
-                event.stopPropagation();
-                event.preventDefault();
-                this.options.onClickLink(href, event);
-              }
               return true;
             }
 

shared/editor/nodes/Attachment.tsx

@@ -4,6 +4,7 @@ import { NodeSpec, NodeType, Node as ProsemirrorNode } from "prosemirror-model";
 import * as React from "react";
 import { Trans } from "react-i18next";
 import { bytesToHumanReadable } from "../../utils/files";
+import { sanitizeHref } from "../../utils/urls";
 import toggleWrap from "../commands/toggleWrap";
 import FileExtension from "../components/FileExtension";
 import Widget from "../components/Widget";
@@ -56,7 +57,7 @@ export default class Attachment extends Node {
           {
             class: `attachment`,
             id: node.attrs.id,
-            href: node.attrs.href,
+            href: sanitizeHref(node.attrs.href),
             download: node.attrs.title,
             "data-size": node.attrs.size,
           },

shared/editor/nodes/Embed.tsx

@@ -2,6 +2,7 @@ import Token from "markdown-it/lib/token";
 import { NodeSpec, NodeType, Node as ProsemirrorNode } from "prosemirror-model";
 import { EditorState } from "prosemirror-state";
 import * as React from "react";
+import { sanitizeHref } from "../../utils/urls";
 import DisabledEmbed from "../components/DisabledEmbed";
 import { MarkdownSerializerState } from "../lib/markdown/serializer";
 import embedsRule from "../rules/embeds";
@@ -47,7 +48,11 @@ export default class Embed extends Node {
       ],
       toDOM: (node) => [
         "iframe",
-        { class: "embed", src: node.attrs.href, contentEditable: "false" },
+        {
+          class: "embed",
+          src: sanitizeHref(node.attrs.href),
+          contentEditable: "false",
+        },
         0,
       ],
       toPlainText: (node) => node.attrs.href,

shared/editor/plugins/PasteHandler.ts

@@ -1,9 +1,9 @@
 import { toggleMark } from "prosemirror-commands";
 import { Plugin } from "prosemirror-state";
 import { isInTable } from "prosemirror-tables";
+import { isUrl } from "../../utils/urls";
 import Extension from "../lib/Extension";
 import isMarkdown from "../lib/isMarkdown";
-import isUrl from "../lib/isUrl";
 import selectionIsInCode from "../queries/isInCode";
 import { LANGUAGES } from "./Prism";
 

shared/i18n/locales/en_US/translation.json

@@ -234,6 +234,7 @@
   "Keep typing to filter": "Keep typing to filter",
   "Open link": "Open link",
   "Go to link": "Go to link",
+  "Sorry, that type of link is not supported": "Sorry, that type of link is not supported",
   "Ordered list": "Ordered list",
   "Page break": "Page break",
   "Paste a link": "Paste a link",

shared/utils/urls.ts

@@ -1,14 +1,26 @@
 import env from "../env";
 import { parseDomain } from "./domains";
 
+/**
+ * Prepends the CDN url to the given path (If a CDN is configured).
+ *
+ * @param path The path to prepend the CDN url to.
+ * @returns The path with the CDN url prepended.
+ */
 export function cdnPath(path: string): string {
   return `${env.CDN_URL}${path}`;
 }
 
-// TODO: HACK: if this is called server-side, it will always return false.
-// - The only call sites to this function and isExternalUrl are on the client
-//   - The reason this is in a shared util is because it's used in an editor plugin
-//     which is also in the shared code
+/**
+ * Returns true if the given string is a link to inside the application.
+ *
+ * Important Note: If this is called server-side, it will always return false.
+ * The reason this is in a shared util is because it's used in an editor plugin
+ * which is also in the shared code
+ *
+ * @param url The url to check.
+ * @returns True if the url is internal, false otherwise.
+ */
 export function isInternalUrl(href: string) {
   // empty strings are never internal
   if (href === "") {
@@ -29,6 +41,50 @@ export function isInternalUrl(href: string) {
   return outline?.host === domain.host;
 }
 
-export function isExternalUrl(href: string) {
-  return !isInternalUrl(href);
+/**
+ * Returns true if the given string is a url.
+ *
+ * @param url The url to check.
+ * @returns True if a url, false otherwise.
+ */
+export function isUrl(text: string) {
+  if (text.match(/\n/)) {
+    return false;
+  }
+
+  try {
+    const url = new URL(text);
+    return url.hostname !== "";
+  } catch (err) {
+    return false;
+  }
+}
+
+/**
+ * Returns true if the given string is a link to outside the application.
+ *
+ * @param url The url to check.
+ * @returns True if the url is external, false otherwise.
+ */
+export function isExternalUrl(url: string) {
+  return !isInternalUrl(url);
+}
+
+/**
+ * For use in the editor, this function will ensure that a link href is
+ * potentially valid, and filter out unsupported and malicious protocols.
+ *
+ * @param href The href to sanitize
+ * @returns The sanitized href
+ */
+export function sanitizeHref(href: string) {
+  if (
+    !isUrl(href) &&
+    !href.startsWith("/") &&
+    !href.startsWith("#") &&
+    !href.startsWith("mailto:")
+  ) {
+    return `https://${href}`;
+  }
+  return href;
 }