From bc6e7679531c4e44a2972730f8f76e6d072c1876 Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 3 Aug 2026 08:22:43 +0200 Subject: [PATCH] prevent XXS in third party apps CommaFeed React client already strips malicious "javascript:" urls but we might as well not store those in the database --- .../main/java/com/commafeed/backend/Urls.java | 26 +++++++++++++++-- .../backend/feed/parser/FeedParser.java | 10 +++---- .../java/com/commafeed/backend/UrlsTest.java | 29 +++++++++++++++++++ 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/commafeed-server/src/main/java/com/commafeed/backend/Urls.java b/commafeed-server/src/main/java/com/commafeed/backend/Urls.java index 3d50ad47..cf54db16 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/Urls.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/Urls.java @@ -8,6 +8,7 @@ import org.netpreserve.urlcanon.Canonicalizer; import org.netpreserve.urlcanon.ParsedUrl; import java.net.URI; +import java.util.Locale; import java.util.regex.Pattern; @UtilityClass @@ -17,17 +18,38 @@ public class Urls { private static final Pattern QUESTION_MARK = Pattern.compile(Pattern.quote("?")); public static boolean isHttp(String url) { - return url.startsWith("http://"); + if (url == null) { + return false; + } + + return url.toLowerCase(Locale.ROOT).startsWith("http://"); } public static boolean isHttps(String url) { - return url.startsWith("https://"); + if (url == null) { + return false; + } + + return url.toLowerCase(Locale.ROOT).startsWith("https://"); } public static boolean isAbsolute(String url) { return isHttp(url) || isHttps(url); } + /** remove malicious 'javascript: 'URLs * */ + public static String sanitize(String url) { + if (url == null) { + return null; + } + + if (!isHttp(url) && !isHttps(url)) { + return null; + } + + return url; + } + /** * @param relativeUrl the url of the entry * @param feedLink the url of the feed as described in the feed diff --git a/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedParser.java b/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedParser.java index dfe7c20e..33a25251 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedParser.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/feed/parser/FeedParser.java @@ -80,8 +80,8 @@ public class FeedParser { handleForeignMarkup(feed); String title = feed.getTitle(); - String link = feed.getLink(); - String iconUrl = feed.getIcon() != null ? feed.getIcon().getUrl() : null; + String link = Urls.sanitize(feed.getLink()); + String iconUrl = Urls.sanitize(feed.getIcon() != null ? feed.getIcon().getUrl() : null); List entries = buildEntries(feed, feedUrl); Instant lastEntryDate = entries.stream().findFirst().map(Entry::published).orElse(null); Instant lastPublishedDate = toValidInstant(feed.getPublishedDate(), false); @@ -145,7 +145,7 @@ public class FeedParser { Instant publishedDate = buildEntryPublishedDate(item); Content content = buildContent(item); - entries.add(new Entry(guid, url, publishedDate, content)); + entries.add(new Entry(guid, Urls.sanitize(url), publishedDate, content)); } entries.sort(ENTRY_COMPARATOR); @@ -173,7 +173,7 @@ public class FeedParser { return null; } - return new Enclosure(enclosure.getUrl(), enclosure.getType()); + return new Enclosure(Urls.sanitize(enclosure.getUrl()), enclosure.getType()); } private Instant buildEntryPublishedDate(SyndEntry item) { @@ -277,7 +277,7 @@ public class FeedParser { return null; } - return new Media(description, thumbnailUrl, thumbnailWidth, thumbnailHeight); + return new Media(description, Urls.sanitize(thumbnailUrl), thumbnailWidth, thumbnailHeight); } private Long averageTimeBetweenEntries(List entries) { diff --git a/commafeed-server/src/test/java/com/commafeed/backend/UrlsTest.java b/commafeed-server/src/test/java/com/commafeed/backend/UrlsTest.java index d7077e75..0ab5ab98 100644 --- a/commafeed-server/src/test/java/com/commafeed/backend/UrlsTest.java +++ b/commafeed-server/src/test/java/com/commafeed/backend/UrlsTest.java @@ -93,6 +93,35 @@ class UrlsTest { "https://www.berliner-zeitung.de/feed.xml")); } + @Test + void testSanitize() { + // valid http/https urls are kept as is + Assertions.assertEquals("http://example.com/foo", Urls.sanitize("http://example.com/foo")); + Assertions.assertEquals( + "https://example.com/foo", Urls.sanitize("https://example.com/foo")); + + // scheme matching is case-insensitive + Assertions.assertEquals("HTTP://example.com/foo", Urls.sanitize("HTTP://example.com/foo")); + Assertions.assertEquals( + "HTTPS://example.com/foo", Urls.sanitize("HTTPS://example.com/foo")); + + // malicious or disallowed schemes are stripped + Assertions.assertNull(Urls.sanitize("javascript:alert(1)")); + Assertions.assertNull(Urls.sanitize("JavaScript:alert(1)")); + Assertions.assertNull(Urls.sanitize("data:text/html,")); + Assertions.assertNull(Urls.sanitize("mailto:foo@example.com")); + Assertions.assertNull(Urls.sanitize("ftp://example.com/foo")); + + // relative and protocol-relative urls are not considered valid + Assertions.assertNull(Urls.sanitize("/blog/entry/1")); + Assertions.assertNull(Urls.sanitize("//example.com/foo")); + + // null and blank input + Assertions.assertNull(Urls.sanitize(null)); + Assertions.assertNull(Urls.sanitize("")); + Assertions.assertNull(Urls.sanitize(" ")); + } + @Test void testRemoveTrailingSlash() { final String url = "http://localhost/";