server side request forgery filter by martyvona · Pull Request #4 · NASA-AMMOS/tile_service

martyvona · 2026-10-12T19:41:38Z

harden URL handling in server side requests

+            if (!isSafeHost(u.getHost())) { //check host of the URL object we actually connect to
+                throw new IOException("blocked request to non-public host " + u.getHost());
+            }
+            HttpURLConnection c = (HttpURLConnection)u.openConnection();


devin-ai-integration

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

Devin Review found 5 potential issues.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

devin-ai-integration · 2026-10-12T19:43:42Z

+            if (!isSafeHost(u.getHost())) { //check host of the URL object we actually connect to
+                throw new IOException("blocked request to non-public host " + u.getHost());
+            }
+            HttpURLConnection c = (HttpURLConnection)u.openConnection();


🟥 DNS rebinding reaches private services

For an attacker-controlled hostname, isSafeHost can approve a public DNS answer before the connection resolves a private one. The request then reaches an internal service despite passing the outbound filter.

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-10-12T19:43:44Z

+    private static boolean isSafeIPv4(byte[] b, int off) {
+        int a0 = b[off] & 0xff, a1 = b[off + 1] & 0xff, a2 = b[off + 2] & 0xff;
+        return !(a0 == 0 || //0.0.0.0/8 "this" network
+                 a0 == 10 || //10.0.0.0/8 private
+                 (a0 == 100 && a1 >= 64 && a1 <= 127) || //100.64.0.0/10 carrier grade NAT, includes Alibaba metadata
+                 a0 == 127 || //127.0.0.0/8 loopback
+                 (a0 == 169 && a1 == 254) || //169.254.0.0/16 link local, includes AWS/GCP/Azure metadata
+                 (a0 == 172 && a1 >= 16 && a1 <= 31) || //172.16.0.0/12 private
+                 (a0 == 192 && a1 == 0 && a2 == 0) || //192.0.0.0/24 IETF protocol assignments, includes OCI metadata
+                 (a0 == 192 && a1 == 168) || //192.168.0.0/16 private
+                 (a0 == 198 && (a1 == 18 || a1 == 19)) || //198.18.0.0/15 benchmarking
+                 a0 >= 224); //224.0.0.0/4 multicast, 240.0.0.0/4 reserved, 255.255.255.255 broadcast


🟥 Non-public IPv4 destinations pass filtering

When a deployment routes reserved IPv4 space internally, isSafeIPv4 accepts addresses outside its listed exclusions. An image URL can then reach services at those non-public addresses.

Was this helpful? React with 👍 or 👎 to provide feedback.

+            if (!isSafeHost(u.getHost())) { //check host of the URL object we actually connect to
+                throw new IOException("blocked request to non-public host " + u.getHost());
+            }
+            HttpURLConnection c = (HttpURLConnection)u.openConnection();


S3 URLs are fetched by the S3 client, never HTTPHelper, so the check did not protect them. With a PrivateLink interface endpoint and private DNS, regional S3 hostnames resolve to VPC addresses, so the check rejected valid images. RDR_URL_WHITELIST_PATTERNS still applies to S3 URLs. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

jl-0

Looks good, added a check in DziTask.java for S3 urls since private VPCs with S3 links is a real concern

…em paths

…ice into feature/ssrf-filter # Conflicts: # src/main/java/jpl/mipl/mars/tile_service/DziTask.java

devin-ai-integration

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

Devin Review found 2 new potential issues.

devin-ai-integration · 2026-10-12T21:29:50Z

+    //[BUCKET.]s3[.-][REGION.]amazonaws.com, also matches e.g. dualstack and website endpoints
+    //host labels must be delimited, e.g. evil-s3amazonaws.com and s3.evilamazonaws.com do not match
+    private static final Pattern S3_HOST_PATTERN =
+        Pattern.compile("([a-z0-9.-]+\\.)?s3[.-]([a-z0-9.-]+\\.)?amazonaws\\.com");


🟡 Trailing-dot S3 images get rejected

For https://bucket.s3.amazonaws.com./image.IMG, isS3Url returns false because its host pattern excludes the DNS root dot. With the default empty whitelist, the service rejects this valid S3 image.

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-10-12T21:29:51Z

DziParams

+                if (url.startsWith("/") || Paths.get(url).isAbsolute()) {
+                    return false;


🟥 Relative paths escape the input directory

With allowDotDotInput enabled, filterUrl accepts a relative ../ path that escapes fs_input_dir. DziParams then prefixes that path without resolving it, exposing files outside the configured input directory.

Was this helpful? React with 👍 or 👎 to provide feedback.

flynnplatt

This all seems fine to me. Devin/Co-Pilot have some thoughts, some of which seem not totally useless so I wouldn't be opposed to you adding some of like the S3 url protections here but I'm also happy to merge as-is

devin-ai-integration

Devin Review found 1 new potential issue.

devin-ai-integration · 2026-10-12T22:09:08Z

DziParams

+            String path = Utils.stripFileScheme(url);
+            try {
+                if (path.startsWith("/") || Paths.get(path).isAbsolute()) {


🟡 File aliases reject root images

For a root-level ICM-test.IMG, filterUrl accepts the bare path but rejects its file:// alias. The default ICM pattern matches the alias's slash, so equivalent image requests return different results.

Learn more

The image filter applies its default ICM exclusion to the URL before removing the file:// prefix. That prefix supplies a slash immediately before a root-level filename, changing whether the pattern matches. DziParams strips the prefix before resolving the input, so both requests target the same file. The ICM check still evaluates the unstripped URL.

Example: With fs_input_dir=test/, ICM-test.IMG passes the default ICM exclusion, but file://ICM-test.IMG matches .*/ICM-[^/]* and returns 400.

Recommended fix: Keep the original URL for whitelist validation, then use the stripped local path consistently for subsequent filename and directory filtering. Add a test comparing the two forms for a root-level ICM image.

Was this helpful? React with 👍 or 👎 to provide feedback.

sonarqubecloud · 2026-10-12T22:19:32Z

Quality Gate failed

Failed conditions
D Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

martyvona added 4 commits October 12, 2026 15:06

add SSRF filter

03e91b3

fix clear-cache.sh

767c2ed

bugfix and update test scripts

c5592ac

add tests for SSRF

ca20f7b

martyvona requested review from flynnplatt and jl-0 October 12, 2026 19:41

martyvona self-assigned this Oct 12, 2026

github-advanced-security Bot found potential problems Oct 12, 2026

View reviewed changes

Comment thread src/main/java/jpl/mipl/mars/tile_service/HTTPHelper.java

if (!isSafeHost(u.getHost())) { //check host of the URL object we actually connect to

throw new IOException("blocked request to non-public host " + u.getHost());

}

HttpURLConnection c = (HttpURLConnection)u.openConnection();

devin-ai-integration Bot reviewed Oct 12, 2026

View reviewed changes

github-advanced-security Bot found potential problems Oct 12, 2026

View reviewed changes

Comment thread src/main/java/jpl/mipl/mars/tile_service/HTTPHelper.java

if (!isSafeHost(u.getHost())) { //check host of the URL object we actually connect to

throw new IOException("blocked request to non-public host " + u.getHost());

}

HttpURLConnection c = (HttpURLConnection)u.openConnection();

address PR feedback

94883d5

This comment was marked as resolved.

Sign in to view

martyvona marked this pull request as draft October 12, 2026 20:26

jl-0 reviewed Oct 12, 2026

View reviewed changes

jl-0 marked this pull request as ready for review October 12, 2026 21:14

martyvona added 3 commits October 12, 2026 17:23

force whitelist for http[s] RDR urls; always reject absolute filesyst…

793ec72

…em paths

Merge branch 'feature/ssrf-filter' of github.com:NASA-AMMOS/tile_serv…

f71e6c0

…ice into feature/ssrf-filter # Conflicts: # src/main/java/jpl/mipl/mars/tile_service/DziTask.java

tweak changelog

5d319d4

devin-ai-integration Bot reviewed Oct 12, 2026

View reviewed changes

github-advanced-security Bot found potential problems Oct 12, 2026

View reviewed changes

Comment thread src/main/java/jpl/mipl/mars/tile_service/S3Helper.java Fixed

tweak comment

f5be7f8

flynnplatt approved these changes Oct 12, 2026

View reviewed changes

fix file:// URLs

3b9eff9

devin-ai-integration Bot reviewed Oct 12, 2026

View reviewed changes

fix potential DoS in slow regex

1a216a5

martyvona merged commit 5ca735d into main Oct 12, 2026
3 of 4 checks passed

		if (url.startsWith("/") \|\| Paths.get(url).isAbsolute()) {
		return false;

Conversation

martyvona commented Oct 12, 2026 • edited by devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devin-ai-integration Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration Bot Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

jl-0 left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flynnplatt left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Oct 12, 2026

Quality Gate failed

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

martyvona commented Oct 12, 2026 •

edited by devin-ai-integration Bot

Loading

devin-ai-integration Bot left a comment •

edited

Loading

devin-ai-integration Bot Oct 12, 2026 •

edited

Loading

devin-ai-integration Bot Oct 12, 2026 •

edited

Loading

devin-ai-integration Bot left a comment •

edited

Loading

devin-ai-integration Bot Oct 12, 2026 •

edited

Loading

devin-ai-integration Bot Oct 12, 2026 •

edited

Loading