Skip to content

Remote marker images: a redirect bypasses the host policy, and the response size is unbounded #170

Description

@jkasprzyk17

Problem

Remote marker images are checked against a host policy that rejects private, loopback and
link-local addresses (Android today; iOS after #166). The check runs once, on the URL the app
passed, and then the request is handed to a client that follows redirects:

// package/android/.../MarkerIconFactory.kt:247-257
remoteMarkerUriRejectReason(image.uri, resolveHostAddress = true)?.let { reason -> … return null }

return try {
  val connection = URL(image.uri).openConnection()
  connection.connectTimeout = 10_000
  connection.readTimeout = 10_000
  connection.getInputStream().use { stream ->
    val bitmap = decodeByteArray(stream.readBytes(), image) ?: return null

HttpURLConnection follows same-protocol redirects by default (instanceFollowRedirects is not
set anywhere), and on iOS URLSession.shared follows them too
(MarkerImageLoader.swift:86). #166 already states this:

Neither platform re-checks a redirect. URLSession and HttpURLConnection both follow 302 by
default, so a public host can redirect to 169.254.169.254. […] it should be fixed on both
sides at once — worth its own issue rather than a one-platform patch here.

Two more holes in the same code:

  • DNS rebinding / TOCTOU: the host is resolved for the check (MarkerIconFactory.kt:460),
    and resolved again by the connection. The second answer is never checked.
  • Unbounded read: stream.readBytes() (Android) and dataTask (iOS) read the whole body
    into memory before decoding. A multi-hundred-megabyte response is an OOM.

Impact

https://allowed.example/pin.png → 302 Location: http://169.254.169.254/… (or any private IP)
is fetched despite the policy. The policy exists to stop exactly that (see the options in #130),
so today it is only a speed bump. The size problem is a crash vector for any app that shows
images from user-controlled URLs.

Where

  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerIconFactory.kt:243-264 — loadRemoteIcon
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerIconFactory.kt:419-500 — policy
  • package/ios/MarkerImageLoader.swift:80-101 — loadRemote via URLSession.shared
  • fix(ios): apply the remote marker image host policy on iOS #166 — the iOS port of the policy, which inherits the redirect gap

Suggested fix

Android

  • (connection as HttpURLConnection).instanceFollowRedirects = false; follow up to N (e.g. 3)
    redirects by hand, running remoteMarkerUriRejectReason on every Location.
  • Connect to the address that passed the check (or re-check connection.url.host after connect).
  • Reject when contentLengthLong exceeds a limit (e.g. 5 MB) and stop reading at the same limit
    when the header is absent.

iOS (on top of #166)

  • A dedicated URLSession with a delegate: urlSession(_:task:willPerformHTTPRedirection:newRequest:)
    runs the policy on the new request and cancels on rejection.
  • Enforce the size limit through expectedContentLength and a streaming data delegate.

Document the limit and the redirect behaviour next to the host policy in the README.

Acceptance criteria

  • A redirect to a rejected host is not followed on either platform, and is logged like a direct rejection
  • A redirect chain longer than the limit is rejected
  • Responses above the size limit are rejected without buffering them whole
  • Ordinary redirects between public hosts (CDNs) still work

Testing

JVM tests with a local MockWebServer (redirect to 127.0.0.1, oversized body, chunked body
without Content-Length); on iOS a URLProtocol stub in the example app or a unit target.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingkotlinThe Kotlin / Android native layer (package/android)platform: androidAffects Androidplatform: iosAffects iOSswiftThe Swift / iOS native layer (package/ios)

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions