fix(ios): apply the remote marker image host policy on iOS

README.md

@@ -514,21 +514,33 @@ Supported image sources:
 | ------------- | ---------------------- | ----------------------------------------- |
 | Bundled asset | `require('./pin.png')` | Resolved on JS side before crossing Nitro |
 | Local URI     | `{ uri: 'file:///…' }` | Platform file paths                       |
-| Remote URL    | `{ uri: 'https://…' }` | Async fetch with in-memory cache; Android restricts the host, see below |
+| Remote URL    | `{ uri: 'https://…' }` | Async fetch with in-memory cache; the host is restricted, see below |
 
-### Remote image hosts on Android
+### Remote image hosts
 
-A remote URL you pass yourself — `image={{ uri: someUrl }}` — is checked before Android fetches
-it. The URL must use `http`/`https`, must not carry `user:password@`, and its host must not
-resolve to a private address: loopback, link-local, site-local, multicast, IPv6 unique-local, a
-`.local`/`.localhost` name, or a cloud metadata endpoint. The address the connection is actually
-made to is checked again before the request is sent, every redirect target is checked the same
-way, and a redirect from `https` to `http` is not followed. This guards the common case of a
-marker icon URL arriving as data from an API. A rejected URL falls back to the default pin and
-logs `Rejected remote marker image URI` under the `NitroMaps` tag.
+A remote URL you pass yourself — `image={{ uri: someUrl }}` — is checked on both platforms before
+the image is fetched. The URL must use `http`/`https`, must not carry `user:password@`, and its
+host must not resolve to a private address: loopback, any-local, link-local, site-local,
+multicast, IPv6 unique-local, a `.local`/`.localhost` name, or a cloud metadata endpoint. Every
+redirect target is checked the same way, so a permitted host cannot forward the fetch to a blocked
+one, and a redirect from `https` to `http` is not followed. This guards the common case of a
+marker icon URL arriving as data from an API.
+
+Android also checks the address the connection is actually made to, before it sends the request.
+iOS checks the name only: `URLSession` resolves it again to connect, so a DNS answer that changes
+in between is not re-checked there. Reaching the network the device itself is on still needs the
+user's Local Network permission on iOS.
+
+A rejected URL logs `Rejected remote marker image URI` — under the `NitroMaps` logcat tag on
+Android, and under the `NitroMaps` category of the `com.nitromaps` subsystem in the unified log
+on iOS. The marker itself falls back to the default pin on both Google providers. On Apple Maps
+it draws with no image at all, the same as any marker image that fails to load.
 
 `require()`d assets are exempt, including the packager URL Metro resolves them to in a
-development build. iOS does not apply this policy today.
+development build.
+
+If you need markers served from a private host, resolve it to an image yourself and pass the
+bytes as a local file, or proxy it through a public endpoint.
 
 Additional props:
 

package/ios/MarkerImageLoader.swift

@@ -1,3 +1,4 @@
+import OSLog
 import UIKit
 
 /// Loads marker images from bundled assets or remote URLs with in-memory caching.
@@ -11,11 +12,30 @@ enum MarkerImageLoader {
     cache.totalCostLimit = maximumCacheBytes
     return cache
   }()
-  private static let session = URLSession.shared
+  /// Not `URLSession.shared`, which cannot carry a delegate: without one the policy would check
+  /// the URL the app supplied and then follow a redirect anywhere, which is the cheapest way to
+  /// defeat it — an attacker-controlled public host answering 302 to a private address.
+  private static let session: URLSession = {
+    URLSession(
+      configuration: .default,
+      delegate: RedirectPolicyDelegate(),
+      delegateQueue: nil
+    )
+  }()
   private static let decodeQueue = DispatchQueue(
     label: "com.nitromaps.markerImageDecode",
     qos: .userInitiated
   )
+  /// Resolving a host blocks, and a slow lookup must not hold up local decoding. Serial rather
+  /// than concurrent so a screen full of markers cannot spawn a thread per lookup; the image
+  /// cache means each distinct URI is resolved once.
+  private static let policyQueue = DispatchQueue(
+    label: "com.nitromaps.markerImagePolicy",
+    qos: .userInitiated
+  )
+  /// The category matches `NITRO_MAPS_LOG_TAG` on Android, so a rejected image is found the same
+  /// way on both platforms.
+  private static let logger = Logger(subsystem: "com.nitromaps", category: "NitroMaps")
 
   static func cachedImage(for image: MarkerImage) -> UIImage? {
     cache.object(forKey: cacheKey(for: image))
@@ -32,7 +52,7 @@ enum MarkerImageLoader {
     }
 
     let uri = image.uri
-    if uri.hasPrefix("http://") || uri.hasPrefix("https://") {
+    if RemoteMarkerUriPolicy.isRemoteUri(uri) {
       loadRemote(uri: uri, cacheKey: cacheKey, image: image, completion: completion)
       return
     }
@@ -52,7 +72,10 @@ enum MarkerImageLoader {
     let width = image.width.map { String($0) } ?? ""
     let height = image.height.map { String($0) } ?? ""
     let scale = image.scale.map { String($0) } ?? ""
-    return "\(image.uri)|\(width)|\(height)|\(scale)" as NSString
+    // `origin` is part of the key so a bundled image cannot warm the cache for a user-supplied
+    // one with the same URI, which would hand it a policy-free entry.
+    let origin = image.origin.map { $0.stringValue } ?? ""
+    return "\(image.uri)|\(width)|\(height)|\(scale)|\(origin)" as NSString
   }
 
   private static func loadLocal(uri: String, image: MarkerImage) -> UIImage? {
@@ -85,12 +108,76 @@ enum MarkerImageLoader {
     image: MarkerImage,
     completion: @escaping (UIImage?) -> Void
   ) {
-    guard let url = URL(string: uri) else {
+    // No policy applies to an image the app bundled, so it must not wait behind the host lookup
+    // of an unrelated one: in a development build every `require()`d marker takes this path.
+    if image.origin == .bundled {
+      guard let url = URL(string: uri) else {
+        completion(nil)
+        return
+      }
+      startRemoteTask(url: url, cacheKey: cacheKey, image: image, completion: completion)
+      return
+    }
+
+    // The pre-check classifies a numeric host literal without touching DNS, so the common
+    // rejection costs nothing and never leaves the caller's thread.
+    if let reason = RemoteMarkerUriPolicy.rejectReason(
+      uri: uri,
+      isBundled: false,
+      resolveHostAddress: false
+    ) {
+      logRejected(uri: uri, reason: reason)
       completion(nil)
       return
     }
 
-    session.dataTask(with: url) { data, _, _ in
+    policyQueue.async {
+      if let reason = RemoteMarkerUriPolicy.rejectReason(
+        uri: uri,
+        isBundled: false,
+        resolveHostAddress: true
+      ) {
+        logRejected(uri: uri, reason: reason)
+        DispatchQueue.main.async { completion(nil) }
+        return
+      }
+
+      guard let url = URL(string: uri) else {
+        DispatchQueue.main.async { completion(nil) }
+        return
+      }
+
+      // URLSession resolves the host again to connect, and it has no hook between that lookup and
+      // the request. So unlike Android, which checks the address it connected to, an answer that
+      // changes after the check above (DNS rebinding) is not re-checked. Reaching the network the
+      // device itself is on still needs the user's Local Network permission.
+      startRemoteTask(url: url, cacheKey: cacheKey, image: image, completion: completion)
+    }
+  }
+
+  /// Logged twice on purpose. A rejected URI can carry the very thing that got it rejected —
+  /// `user:password@` is one of the reasons — and a query string can hold a signed token, so the
+  /// public half is stripped of both. The full URI follows as private: the unified log keeps it
+  /// out of a shipped app's logs, and a developer attached to the simulator or a debugger still
+  /// sees it. That matters because Metro puts the asset's identity in the query string, so the
+  /// stripped form alone does not say which image was refused.
+  private static func logRejected(uri: String, reason: String) {
+    logger.warning(
+      """
+      Rejected remote marker image URI (\(reason, privacy: .public)): \
+      \(RemoteMarkerUriPolicy.loggableURI(uri), privacy: .public) \
+      [full: \(uri, privacy: .private)]
+      """
+    )
+  }
+
+  private static func startRemoteTask(
+    url: URL,
+    cacheKey: NSString,
+    image: MarkerImage,
+    completion: @escaping (UIImage?) -> Void
+  ) {
+    let task = session.dataTask(with: url) { data, _, _ in
       let uiImage: UIImage?
       if let data, let decoded = UIImage(data: data) {
         uiImage = resize(decoded, image: image)
@@ -104,7 +191,59 @@ enum MarkerImageLoader {
       DispatchQueue.main.async {
         completion(uiImage)
       }
-    }.resume()
+    }
+    // The delegate sees tasks, not images, so the exemption has to travel with the task.
+    task.taskDescription = image.origin == .bundled ? bundledTaskDescription : nil
+    task.resume()
+  }
+
+  private static let bundledTaskDescription = "com.nitromaps.bundledMarkerImage"
+
+  /// Re-runs the host policy on every redirect destination. Without it the policy would only ever
+  /// see the URL the app supplied.
+  private final class RedirectPolicyDelegate: NSObject, URLSessionTaskDelegate {
+    func urlSession(
+      _ session: URLSession,
+      task: URLSessionTask,
+      willPerformHTTPRedirection response: HTTPURLResponse,
+      newRequest request: URLRequest,
+      completionHandler: @escaping (URLRequest?) -> Void
+    ) {
+      guard let destination = request.url else {
+        completionHandler(nil)
+        return
+      }
+
+      if let source = response.url,
+        let reason = RemoteMarkerUriPolicy.redirectRejectReason(from: source, to: destination)
+      {
+        MarkerImageLoader.logRejected(uri: destination.absoluteString, reason: reason)
+        completionHandler(nil)
+        return
+      }
+
+      if task.taskDescription == MarkerImageLoader.bundledTaskDescription {
+        completionHandler(request)
+        return
+      }
+
+      let uri = destination.absoluteString
+
+      // Off the delegate queue: resolving blocks, and this queue also delivers completions.
+      MarkerImageLoader.policyQueue.async {
+        if let reason = RemoteMarkerUriPolicy.rejectReason(
+          uri: uri,
+          isBundled: false,
+          resolveHostAddress: true
+        ) {
+          MarkerImageLoader.logRejected(uri: uri, reason: "redirect: \(reason)")
+          // `nil` stops the redirect; the task completes with the redirect response body.
+          completionHandler(nil)
+          return
+        }
+        completionHandler(request)
+      }
+    }
   }
 
   /// Decoded size in bytes, so the cache evicts by memory rather than by count.

package/ios/Package.swift

@@ -28,6 +28,10 @@ let package = Package(
       name: "NitroMapsShapeDiff",
       path: "ShapeDiff"
     ),
+    .target(
+      name: "NitroMapsRemoteImagePolicy",
+      path: "RemoteImagePolicy"
+    ),
     .testTarget(
       name: "NitroMapsColorParserTests",
       dependencies: ["NitroMapsColorParser"],
@@ -54,5 +58,10 @@ let package = Package(
       dependencies: ["NitroMapsShapeDiff"],
       path: "Tests/ShapeDiff"
     ),
+    .testTarget(
+      name: "NitroMapsRemoteImagePolicyTests",
+      dependencies: ["NitroMapsRemoteImagePolicy"],
+      path: "Tests/RemoteImagePolicy"
+    ),
   ]
 )