Skip to content

(CodeQL) Sandboxed URL creation to prevent SSRF attacks #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pixeebot-edge wants to merge 1 commit into
base: main
Choose a base branch
from
+14 −2
Open

(CodeQL) Sandboxed URL creation to prevent SSRF attacks #34

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions app/src/main/java/org/apache/roller/planet/business/fetcher/RomeFeedFetcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,15 @@
throw new IllegalArgumentException("feed url cannot be null");
}

try {
URI uri = new URI(feedURL);
if (!uri.isAbsolute() || uri.getHost() == null) {
throw new FetcherException("Invalid URL - " + feedURL);
}
} catch (Exception e) {
throw new FetcherException("Invalid URL format - " + feedURL, e);
}

// fetch the feed
log.debug("Fetching feed: "+feedURL);
SyndFeed feed;
Expand Down Expand Up @@ -226,8 +235,11 @@
}

private SyndFeed fetchFeed(String url) throws IOException, InterruptedException, FeedException {

HttpRequest request = requestBuilder.copy().uri(URI.create(url)).build();
URI uri = URI.create(url);
if (!"REPLACE_WITH_ALLOWED_HOST".equalsIgnoreCase(uri.getHost())) {
throw new IllegalArgumentException("Invalid host");
}
HttpRequest request = requestBuilder.copy().uri(uri).build();

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

Potential server-side request forgery due to a
user-provided value
Loading
.

Copilot Autofix

AI 7 months ago

To fix the SSRF vulnerability, we need to validate the user-provided URL against a predefined list of allowed hosts or URL prefixes. This ensures that the server only makes requests to trusted endpoints. The best approach is to maintain a list of allowed hosts in the configuration and validate the uri.getHost() against this list before constructing the HTTP request.

Steps to implement the fix:

  1. Define a list of allowed hosts as a constant or retrieve it from a configuration file.
  2. Replace the placeholder "REPLACE_WITH_ALLOWED_HOST" with logic to check if the host of the URI is in the list of allowed hosts.
  3. Throw an exception if the host is not in the allowed list.
  4. Ensure that the validation is applied consistently across all methods that use the fetchFeed function.
Suggested changeset 2
app/src/main/java/org/apache/roller/planet/business/fetcher/RomeFeedFetcher.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/src/main/java/org/apache/roller/planet/business/fetcher/RomeFeedFetcher.java b/app/src/main/java/org/apache/roller/planet/business/fetcher/RomeFeedFetcher.java
--- a/app/src/main/java/org/apache/roller/planet/business/fetcher/RomeFeedFetcher.java
+++ b/app/src/main/java/org/apache/roller/planet/business/fetcher/RomeFeedFetcher.java
@@ -238,4 +238,5 @@
         URI uri = URI.create(url);
-        if (!"REPLACE_WITH_ALLOWED_HOST".equalsIgnoreCase(uri.getHost())) {
-            throw new IllegalArgumentException("Invalid host");
+        List<String> allowedHosts = List.of("trustedhost1.com", "trustedhost2.com");
+        if (!allowedHosts.contains(uri.getHost())) {
+            throw new IllegalArgumentException("Invalid host: " + uri.getHost());
         }
EOF
@@ -238,4 +238,5 @@
URI uri = URI.create(url);
if (!"REPLACE_WITH_ALLOWED_HOST".equalsIgnoreCase(uri.getHost())) {
throw new IllegalArgumentException("Invalid host");
List<String> allowedHosts = List.of("trustedhost1.com", "trustedhost2.com");
if (!allowedHosts.contains(uri.getHost())) {
throw new IllegalArgumentException("Invalid host: " + uri.getHost());
}
app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java
--- a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java
+++ b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java
@@ -187,3 +187,7 @@
                     FeedFetcher fetcher = WebloggerFactory.getWeblogger().getFeedFetcher();
-                    sub = fetcher.fetchSubscription(getSubUrl());
+                    String url = getSubUrl();
+                    if (url == null || url.isEmpty()) {
+                        throw new IllegalArgumentException("Subscription URL cannot be null or empty");
+                    }
+                    sub = fetcher.fetchSubscription(url);
 
EOF
@@ -187,3 +187,7 @@
FeedFetcher fetcher = WebloggerFactory.getWeblogger().getFeedFetcher();
sub = fetcher.fetchSubscription(getSubUrl());
String url = getSubUrl();
if (url == null || url.isEmpty()) {
throw new IllegalArgumentException("Subscription URL cannot be null or empty");
}
sub = fetcher.fetchSubscription(url);

Copilot is powered by AI and may make mistakes. Always verify output.

try(XmlReader reader = new XmlReader(client.send(request, ofInputStream()).body())) {
return new SyndFeedInput().build(reader);
Expand Down
Loading