From 8a67b5129e2011e37fdf49356058f8bd69524a39 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Wed, 1 Jul 2026 10:28:33 -0400 Subject: [PATCH] Strip unsafe URLs from Markdown links --- docs/releases.md | 1 + web/src/app/utils.js | 11 ++++++++ web/src/app/utils.test.js | 38 ++++++++++++++++++++++++++ web/src/components/MarkdownContent.jsx | 16 ++++++++++- 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/docs/releases.md b/docs/releases.md index 527c52dd..8f5a6ebc 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1994,6 +1994,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release * Web app: Smooth transitions and loading animation, remove flickering * Web app: `GET /account` now reads from the primary database instead of a read replica, so the account view no longer shows stale data right after a change when replicas lag behind * Docs: Document the third-party HelmForge Helm chart as a Kubernetes installation option ([#1727](https://github.com/binwiederhier/ntfy/issues/1727), thanks to [@mberlofa](https://github.com/mberlofa)) +* Web app: Strip unsafe URL protocols (`javascript:`, `data:`, ...) from links and images in Markdown-rendered messages, so they no longer trigger an uncaught "React has blocked a javascript: URL" error (thanks to [@jvoisin](https://github.com/jvoisin) for reporting) ### ntfy Android v1.25.x (UNRELEASED) diff --git a/web/src/app/utils.js b/web/src/app/utils.js index a33fe258..1ea39541 100644 --- a/web/src/app/utils.js +++ b/web/src/app/utils.js @@ -444,6 +444,17 @@ export const updateFavicon = async (count) => { } }; +// Matches a URL whose scheme is NOT in the safe list (https, mailto, ...). A scheme +// (RFC 3986: [a-z][a-z0-9+.-]*) can't contain a slash, query, or hash, so a colon later +// in the path/query (e.g. foo?x=a:b) is never mistaken for a protocol separator. Relative +// URLs have no scheme and never match. This strips javascript:/data:/vbscript:/... so React +// never has to block a javascript: URL at render time -- which it does loudly, throwing an +// uncaught error. +const unsafeUrlProtocol = /^\s*(?!(?:https?|ftps?|ircs?|mailto|xmpp|tel):)[a-z][a-z0-9+.-]*:/i; + +// Returns the URL unchanged if it uses a safe protocol or is relative, otherwise "". +export const sanitizeUrl = (url) => (url && unsafeUrlProtocol.test(url) ? "" : url || ""); + export const copyToClipboard = (text) => { if (navigator.clipboard && window.isSecureContext) { return navigator.clipboard.writeText(text); diff --git a/web/src/app/utils.test.js b/web/src/app/utils.test.js index cbf4b0d0..a1f2a61a 100644 --- a/web/src/app/utils.test.js +++ b/web/src/app/utils.test.js @@ -19,6 +19,7 @@ import { withBearerAuth, maybeWithAuth, splitNoEmpty, + sanitizeUrl, hashCode, formatBytes, formatNumber, @@ -216,3 +217,40 @@ describe("date/time formatting", () => { expect(formatDateTime(ts, DATE_FORMAT.ISO8601, TIME_FORMAT.H12)).toBe("2026-03-08 14:30"); }); }); + +describe("sanitizeUrl", () => { + it("keeps safe absolute URLs", () => { + expect(sanitizeUrl("https://ntfy.sh")).toBe("https://ntfy.sh"); + expect(sanitizeUrl("http://example.com/foo?bar=1")).toBe("http://example.com/foo?bar=1"); + expect(sanitizeUrl("mailto:phil@ntfy.sh")).toBe("mailto:phil@ntfy.sh"); + expect(sanitizeUrl("ftp://ftp.example.com/file.txt")).toBe("ftp://ftp.example.com/file.txt"); + expect(sanitizeUrl("ftps://ftp.example.com/file.txt")).toBe("ftps://ftp.example.com/file.txt"); + }); + + it("keeps relative URLs, fragments and query strings", () => { + expect(sanitizeUrl("/docs")).toBe("/docs"); + expect(sanitizeUrl("./foo")).toBe("./foo"); + expect(sanitizeUrl("#section")).toBe("#section"); + expect(sanitizeUrl("?q=1")).toBe("?q=1"); + expect(sanitizeUrl("foo/bar")).toBe("foo/bar"); + }); + + it("does not treat a colon after a slash/query/hash as a protocol", () => { + expect(sanitizeUrl("/path:with:colons")).toBe("/path:with:colons"); + expect(sanitizeUrl("foo?x=a:b")).toBe("foo?x=a:b"); + }); + + it("strips dangerous protocols", () => { + expect(sanitizeUrl("javascript:alert(document.domain)")).toBe(""); + expect(sanitizeUrl("JavaScript:alert(1)")).toBe(""); + expect(sanitizeUrl(" javascript:alert(1)")).toBe(""); + expect(sanitizeUrl("vbscript:msgbox(1)")).toBe(""); + expect(sanitizeUrl("data:text/html,")).toBe(""); + }); + + it("handles empty and nullish input", () => { + expect(sanitizeUrl("")).toBe(""); + expect(sanitizeUrl(undefined)).toBe(""); + expect(sanitizeUrl(null)).toBe(""); + }); +}); diff --git a/web/src/components/MarkdownContent.jsx b/web/src/components/MarkdownContent.jsx index ec6ca21f..12a66283 100644 --- a/web/src/components/MarkdownContent.jsx +++ b/web/src/components/MarkdownContent.jsx @@ -2,6 +2,18 @@ import * as React from "react"; import { useEffect } from "react"; import { useRemark } from "react-remark"; import styled from "@emotion/styled"; +import { sanitizeUrl } from "../app/utils"; + +// Strip unsafe URL protocols (javascript:, data:, ...) from links and images. Without +// this, React blocks javascript: URLs at render time and throws an uncaught "React has +// blocked a javascript: URL" error. +const SafeLink = ({ href, children, ...props }) => ( + + {children} + +); + +const SafeImage = ({ src, alt, ...props }) => {alt}; const MarkdownContainer = styled("div")` line-height: 1; @@ -50,7 +62,9 @@ const MarkdownContainer = styled("div")` // dependency stack) is heavy, so this component lives in its own module and is loaded // lazily by Notifications.jsx -- it only ships when a markdown message is actually shown. const MarkdownContent = ({ content }) => { - const [reactContent, setMarkdownSource] = useRemark(); + const [reactContent, setMarkdownSource] = useRemark({ + rehypeReactOptions: { components: { a: SafeLink, img: SafeImage } }, + }); useEffect(() => { setMarkdownSource(content);