mirror of
https://github.com/Athou/commafeed.git
synced 2026-09-24 21:15:13 +00:00
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
This commit is contained in:
@@ -8,6 +8,7 @@ import org.netpreserve.urlcanon.Canonicalizer;
|
|||||||
import org.netpreserve.urlcanon.ParsedUrl;
|
import org.netpreserve.urlcanon.ParsedUrl;
|
||||||
|
|
||||||
import java.net.URI;
|
import java.net.URI;
|
||||||
|
import java.util.Locale;
|
||||||
import java.util.regex.Pattern;
|
import java.util.regex.Pattern;
|
||||||
|
|
||||||
@UtilityClass
|
@UtilityClass
|
||||||
@@ -17,17 +18,38 @@ public class Urls {
|
|||||||
private static final Pattern QUESTION_MARK = Pattern.compile(Pattern.quote("?"));
|
private static final Pattern QUESTION_MARK = Pattern.compile(Pattern.quote("?"));
|
||||||
|
|
||||||
public static boolean isHttp(String url) {
|
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) {
|
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) {
|
public static boolean isAbsolute(String url) {
|
||||||
return isHttp(url) || isHttps(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 relativeUrl the url of the entry
|
||||||
* @param feedLink the url of the feed as described in the feed
|
* @param feedLink the url of the feed as described in the feed
|
||||||
|
|||||||
@@ -80,8 +80,8 @@ public class FeedParser {
|
|||||||
handleForeignMarkup(feed);
|
handleForeignMarkup(feed);
|
||||||
|
|
||||||
String title = feed.getTitle();
|
String title = feed.getTitle();
|
||||||
String link = feed.getLink();
|
String link = Urls.sanitize(feed.getLink());
|
||||||
String iconUrl = feed.getIcon() != null ? feed.getIcon().getUrl() : null;
|
String iconUrl = Urls.sanitize(feed.getIcon() != null ? feed.getIcon().getUrl() : null);
|
||||||
List<Entry> entries = buildEntries(feed, feedUrl);
|
List<Entry> entries = buildEntries(feed, feedUrl);
|
||||||
Instant lastEntryDate = entries.stream().findFirst().map(Entry::published).orElse(null);
|
Instant lastEntryDate = entries.stream().findFirst().map(Entry::published).orElse(null);
|
||||||
Instant lastPublishedDate = toValidInstant(feed.getPublishedDate(), false);
|
Instant lastPublishedDate = toValidInstant(feed.getPublishedDate(), false);
|
||||||
@@ -145,7 +145,7 @@ public class FeedParser {
|
|||||||
Instant publishedDate = buildEntryPublishedDate(item);
|
Instant publishedDate = buildEntryPublishedDate(item);
|
||||||
Content content = buildContent(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);
|
entries.sort(ENTRY_COMPARATOR);
|
||||||
@@ -173,7 +173,7 @@ public class FeedParser {
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
return new Enclosure(enclosure.getUrl(), enclosure.getType());
|
return new Enclosure(Urls.sanitize(enclosure.getUrl()), enclosure.getType());
|
||||||
}
|
}
|
||||||
|
|
||||||
private Instant buildEntryPublishedDate(SyndEntry item) {
|
private Instant buildEntryPublishedDate(SyndEntry item) {
|
||||||
@@ -277,7 +277,7 @@ public class FeedParser {
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
return new Media(description, thumbnailUrl, thumbnailWidth, thumbnailHeight);
|
return new Media(description, Urls.sanitize(thumbnailUrl), thumbnailWidth, thumbnailHeight);
|
||||||
}
|
}
|
||||||
|
|
||||||
private Long averageTimeBetweenEntries(List<Entry> entries) {
|
private Long averageTimeBetweenEntries(List<Entry> entries) {
|
||||||
|
|||||||
@@ -93,6 +93,35 @@ class UrlsTest {
|
|||||||
"https://www.berliner-zeitung.de/feed.xml"));
|
"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,<script>alert(1)</script>"));
|
||||||
|
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
|
@Test
|
||||||
void testRemoveTrailingSlash() {
|
void testRemoveTrailingSlash() {
|
||||||
final String url = "http://localhost/";
|
final String url = "http://localhost/";
|
||||||
|
|||||||
Reference in New Issue
Block a user