Skip to content

fix(data): prevent SSRF from untrusted KML/GeoJSON icon and GroundOverlay href - #1758

Merged
dkhawk merged 2 commits into
googlemaps:mainfrom
herdiyana256:fix/kml-geojson-icon-href-ssrf
Aug 26, 2026
Merged

fix(data): prevent SSRF from untrusted KML/GeoJSON icon and GroundOverlay href#1758
dkhawk merged 2 commits into
googlemaps:mainfrom
herdiyana256:fix/kml-geojson-icon-href-ssrf

Conversation

@herdiyana256

Copy link
Copy Markdown
Contributor

Summary

Icon and GroundOverlay <href> URLs are read from the parsed KML/GeoJSON document and fetched without any validation, giving a crafted (untrusted) document a Server-Side Request Forgery (SSRF) primitive.

  • Style/IconStyle/Icon/hrefKmlStyle.iconUrl
  • GroundOverlay/Icon/hrefKmlGroundOverlay.imageUrl

Both flow to UrlIconProvider.loadBitmapFromUrl, which did:

val url = URL(urlString)
val connection = url.openConnection() as HttpURLConnection
connection.doInput = true
connection.connect()          // <-- request issued; no validation anywhere

A malicious KML/GeoJSON (downloaded, shared, or user-provided) can therefore make the app issue requests to loopback, link-local (169.254.169.254 metadata), and private-network hosts reachable from the device. HttpURLConnection also follows redirects by default, so a public-looking href can 302 to an internal target.

The KmlUrlSanitizer interface exists but sanitizeUrl(...) is never called anywhere in the codebase (dead code), and KmlLayer/GeoJsonLayer hard-code UrlIconProvider() with no way to inject one — so there was no protection by default and no way for a developer to add one.

Reproduction (network sequence is verbatim from loadBitmapFromUrl)

A KML with <Icon><href>http://127.0.0.1:8474/INTERNAL</href></Icon> (or a 169.254.169.254 / RFC1918 host) causes a real request to that host; a redirector href is followed to the internal target. Confirmed against a local listener that logged the inbound requests.

Fix

  • DefaultKmlUrlSanitizer (new): allows only http/https whose host does not resolve to a loopback / any-local / link-local / site-local (RFC 1918) / multicast address; returns null (block) otherwise. Public icon/overlay URLs are unaffected.
  • UrlIconProvider now applies a KmlUrlSanitizer (secure default) before every fetch — blocked URLs issue no request — and sets instanceFollowRedirects = false to stop redirect-based bypass. Callers may pass a custom sanitizer, or null to opt out.
  • Hermetic unit tests for the sanitizer (IP-literal hosts, no DNS/network).

This makes the previously-dead KmlUrlSanitizer functional and the default behavior secure, without changing the public KmlLayer/GeoJsonLayer API. Verified on a JVM: internal/metadata/RFC1918/redirector/file:// hrefs are blocked with no request issued, while a public host still loads.

Scope / notes

  • DNS-rebinding between the sanitizer's resolution and the connection's resolution is a known residual limitation of host-based validation; disabling auto-redirects and failing closed on unresolved hosts substantially reduces the attack surface.

…rlay href

Icon (IconStyle/Icon/href) and GroundOverlay (Icon/href) URLs are read from the
parsed KML/GeoJSON document, which is frequently untrusted (downloaded, shared,
or user-provided). UrlIconProvider.loadBitmapFromUrl fetched these URLs verbatim
(URL(href).openConnection().connect()) with no validation, giving a crafted
document an SSRF primitive against loopback, link-local (169.254.169.254 metadata),
and private-network hosts reachable from the device. The existing KmlUrlSanitizer
interface was never invoked anywhere (dead code) and could not be injected through
KmlLayer/GeoJsonLayer, so there was no protection by default.

- Add DefaultKmlUrlSanitizer: allows only http/https whose host does not resolve to
  a loopback/any-local/link-local/site-local/multicast address; blocks otherwise.
- UrlIconProvider now applies a KmlUrlSanitizer (secure default) before every fetch
  and disables auto-redirects to stop redirect-based SSRF bypass. Callers may pass a
  custom sanitizer, or null to opt out. Public icon URLs are unaffected.
- Add hermetic unit tests for the sanitizer.
@dkhawk
dkhawk requested a review from kikoso August 25, 2026 15:43
Comment thread data/src/main/java/com/google/maps/android/data/kml/DefaultKmlUrlSanitizer.java Outdated
Comment thread data/src/main/java/com/google/maps/android/data/renderer/UrlIconProvider.kt Outdated
@kikoso

kikoso commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Hi @herdiyana256 ! Thanks for the contribution, this fixes a real issue.

I have left a few comments, let me know if you want to discuss any of them.

…alidation, @jvmoverloads

- DefaultKmlUrlSanitizer: add the internal ranges InetAddress does not classify:
  fc00::/7 (IPv6 ULA, RFC 4193), 100.64.0.0/10 (CGNAT, RFC 6598) and
  192.0.0.0/24 (IETF protocol assignments, RFC 6890). Factored the check into a
  reusable static isInternalAddress(InetAddress) and fail closed on empty
  resolution.
- UrlIconProvider: instead of disabling redirects outright (which silently broke
  legitimate redirecting icon URLs, e.g. CDNs and signed S3/GCS links), follow up
  to 5 redirects manually, re-running each hop's target through the sanitizer
  before connecting, and check responseCode so non-2xx no longer yields a silent
  empty decode. Added @jvmoverloads to the constructor for the new parameter.
- Tests: cover the newly blocked ranges (CGNAT, IETF, IPv6 ULA) and public
  addresses just outside CGNAT.
@herdiyana256

herdiyana256 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Replied on each thread above. Short version: extra ranges (fc00::/7, 100.64.0.0/10, 192.0.0.0/24) covered, @jvmoverloads added, and redirects are now followed manually with a per-hop re-sanitize and a responseCode check. DNS-rebinding pin is the one open item, left a question about the approach on that thread. The failing check is the jacoco coverage-comment step missing a token on fork PRs, not a test failure.

@kikoso

kikoso commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Thanks! Yeah, don't worry about the test flow, it doesn't run for remote contributors.

@kikoso kikoso left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@herdiyana256

Copy link
Copy Markdown
Contributor Author

Thanks for the review and approval! Looks like the workflows are just waiting on a maintainer to approve the run for this fork PR. Whenever one of you has a moment to kick that off and merge, I'm happy to rebase if needed.

@kikoso

kikoso commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Ignore the reporting flow, doesn't work with external contributions

@dkhawk
dkhawk merged commit 9078c33 into googlemaps:main Aug 26, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants