Skip to content

Commit

Permalink
Improved sanitization of href's in editor
Browse files Browse the repository at this point in the history
  • Loading branch information
tommoor committed Jul 6, 2022
1 parent 8a29a35 commit 0e93732
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 41 deletions.
15 changes: 2 additions & 13 deletions app/editor/components/LinkEditor.tsx
Expand Up @@ -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";
Expand Down Expand Up @@ -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 });
};
Expand Down
1 change: 1 addition & 0 deletions app/hooks/useDictionary.ts
Expand Up @@ -52,6 +52,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")}…`,
Expand Down
12 changes: 0 additions & 12 deletions shared/editor/lib/isUrl.ts

This file was deleted.

30 changes: 19 additions & 11 deletions shared/editor/marks/Link.tsx
Expand Up @@ -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";
Expand Down Expand Up @@ -80,6 +80,7 @@ export default class Link extends Mark {
"a",
{
...node.attrs,
href: sanitizeHref(node.attrs.href),
rel: "noopener noreferrer nofollow",
},
0,
Expand Down Expand Up @@ -193,18 +194,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);
if (this.options.onClickLink) {
event.stopPropagation();
event.preventDefault();
this.options.onClickLink(href, event);
}
} catch (err) {
this.editor.props.onShowToast(
this.options.dictionary.openLinkError
);
}

return true;
}

Expand Down
3 changes: 2 additions & 1 deletion shared/editor/nodes/Attachment.tsx
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
},
Expand Down
7 changes: 6 additions & 1 deletion shared/editor/nodes/Embed.tsx
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion 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";

Expand Down
1 change: 1 addition & 0 deletions shared/i18n/locales/en_US/translation.json
Expand Up @@ -231,6 +231,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",
Expand Down
64 changes: 62 additions & 2 deletions shared/utils/urls.ts
Expand Up @@ -2,10 +2,26 @@ import { parseDomain } from "./domains";

const env = typeof window !== "undefined" ? window.env : process.env;

/**
* 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}`;
}

/**
* 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) {
if (href[0] === "/") {
return true;
Expand All @@ -29,6 +45,50 @@ export function isInternalUrl(href: string) {
return false;
}

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;
}

0 comments on commit 0e93732

Please sign in to comment.