Improving the urls to not break protocols and adding tests

shared/constants.ts

@@ -1,3 +1,10 @@
 export const USER_PRESENCE_INTERVAL = 5000;
 
 export const MAX_AVATAR_DISPLAY = 6;
+
+export const BLACKLIST_PROTOCOLS = [
+  "javascript:",
+  "file:",
+  "vbscript:",
+  "data:",
+];

shared/utils/urls.test.ts

@@ -1,5 +1,100 @@
+import * as urlsUtils from "./urls";
 import { urlRegex } from "./urls";
 
+describe("IsUrl Method", () => {
+  describe("invalid urls", () => {
+    it("should return false", () => {
+      expect(urlsUtils.isUrl("")).toBe(false);
+      expect(urlsUtils.isUrl("#invalidurl")).toBe(false);
+      expect(urlsUtils.isUrl("mailto:")).toBe(false);
+      expect(urlsUtils.isUrl("://")).toBe(false);
+    });
+  });
+});
+
+describe("isInternalUrl Method", () => {
+  it("should return false if empty string", () => {
+    expect(urlsUtils.isInternalUrl("")).toBe(false);
+  });
+
+  it("should return true if starting with relative path", () => {
+    expect(urlsUtils.isInternalUrl("/drafts")).toEqual(true);
+  });
+});
+
+describe("isExternalUrl Method", () => {
+  it("should return false if empty url", () => {
+    expect(urlsUtils.isExternalUrl("")).toBe(false);
+  });
+
+  it("should return false if internal url", () => {
+    expect(urlsUtils.isExternalUrl("/drafts")).toBe(false);
+  });
+});
+
+describe("sanitizeUrl Method", () => {
+  it("should return undefined if not url", () => {
+    expect(urlsUtils.sanitizeUrl(undefined)).toBeUndefined();
+    expect(urlsUtils.sanitizeUrl(null)).toBeUndefined();
+    expect(urlsUtils.sanitizeUrl("")).toBeUndefined();
+  });
+
+  it("should append https:// to non-special urls", () => {
+    expect(urlsUtils.sanitizeUrl("www.google.com")).toEqual(
+      "https://www.google.com"
+    );
+  });
+
+  describe("special urls", () => {
+    it("should return the url as it's if starting with /", () => {
+      expect(urlsUtils.sanitizeUrl("/drafts")).toEqual("/drafts");
+    });
+    it("should return the url as it's if starting with #", () => {
+      expect(urlsUtils.sanitizeUrl("#home")).toEqual("#home");
+    });
+    it("should return the url as it's if it's mailto:", () => {
+      expect(urlsUtils.sanitizeUrl("mailto:outline@getoutline.com")).toEqual(
+        "mailto:outline@getoutline.com"
+      );
+    });
+    it("should return the url as it's if it's mailto:", () => {
+      expect(urlsUtils.sanitizeUrl("mailto:outline@getoutline.com")).toEqual(
+        "mailto:outline@getoutline.com"
+      );
+    });
+    it("should return the url as it's if it's sms:, fax:, tel:", () => {
+      expect(urlsUtils.sanitizeUrl("mailto:outline@getoutline.com")).toEqual(
+        "mailto:outline@getoutline.com"
+      );
+      expect(urlsUtils.sanitizeUrl("tel:0123456789")).toEqual("tel:0123456789");
+      expect(urlsUtils.sanitizeUrl("fax:0123456789")).toEqual("fax:0123456789");
+      expect(urlsUtils.sanitizeUrl("sms:0123456789")).toEqual("sms:0123456789");
+    });
+    it("should return the url as it's if it's a special domain", () => {
+      expect(urlsUtils.sanitizeUrl("mqtt://getoutline.com")).toEqual(
+        "mqtt://getoutline.com"
+      );
+    });
+  });
+
+  describe("Blocked protocols", () => {
+    it("should be sanitized", () => {
+      expect(urlsUtils.sanitizeUrl("file://localhost.com/outline.txt")).toEqual(
+        "https://file://localhost.com/outline.txt"
+      );
+      expect(urlsUtils.sanitizeUrl("javascript:whatever")).toEqual(
+        "https://javascript:whatever"
+      );
+      expect(
+        urlsUtils.sanitizeUrl("data:text/html,<script>alert('hi');</script>")
+      ).toEqual("https://data:text/html,<script>alert('hi');</script>");
+      expect(urlsUtils.sanitizeUrl("vbscript:whatever")).toEqual(
+        "https://vbscript:whatever"
+      );
+    });
+  });
+});
+
 describe("#urlRegex", () => {
   it("should return undefined for invalid urls", () => {
     expect(urlRegex(undefined)).toBeUndefined();

shared/utils/urls.ts

@@ -1,4 +1,5 @@
 import { escapeRegExp } from "lodash";
+import { BLACKLIST_PROTOCOLS } from "../constants";
 import env from "../env";
 import { parseDomain } from "./domains";
 
@@ -55,7 +56,7 @@ export function isUrl(text: string) {
 
   try {
     const url = new URL(text);
-    return url.hostname !== "";
+    return url.hostname !== "" && !BLACKLIST_PROTOCOLS.includes(url.protocol);
   } catch (err) {
     return false;
   }
@@ -68,7 +69,7 @@ export function isUrl(text: string) {
  * @returns True if the url is external, false otherwise.
  */
 export function isExternalUrl(url: string) {
-  return !isInternalUrl(url);
+  return !!url && !isInternalUrl(url);
 }
 
 /**
@@ -87,7 +88,10 @@ export function sanitizeUrl(url: string | null | undefined) {
     !isUrl(url) &&
     !url.startsWith("/") &&
     !url.startsWith("#") &&
-    !url.startsWith("mailto:")
+    !url.startsWith("mailto:") &&
+    !url.startsWith("sms:") &&
+    !url.startsWith("fax:") &&
+    !url.startsWith("tel:")
   ) {
     return `https://${url}`;
   }