fix(llm-access): block DNS-rebinding SSRF in kiro remote-media fetch

llm-access/src/provider.rs

@@ -496,6 +496,7 @@ static KIRO_REMOTE_MEDIA_CLIENT: std::sync::LazyLock<reqwest::Client> =
         reqwest::Client::builder()
             .timeout(KIRO_REMOTE_MEDIA_TIMEOUT)
             .redirect(reqwest::redirect::Policy::none())
+            .dns_resolver(Arc::new(kiro_media::PrivateFilteringDnsResolver))
             .pool_idle_timeout(provider_client_pool_idle_timeout())
             .pool_max_idle_per_host(provider_client_pool_max_idle_per_host())
             .tcp_keepalive(Duration::from_secs(30))

llm-access/src/provider/kiro_media.rs

@@ -1,6 +1,6 @@
 //! Kiro remote-media (image/document) resolution, validation, and fetch.
 
-use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
+use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
 
 use async_trait::async_trait;
 use base64::Engine as _;
@@ -459,32 +459,47 @@ fn validate_kiro_remote_media_url(
             ))
         },
     }
-    let host = url
-        .host_str()
-        .ok_or_else(|| KiroRemoteMediaResolutionError::new("URL source is missing host"))?;
-    if host.eq_ignore_ascii_case("localhost") || host.ends_with(".localhost") {
-        return Err(KiroRemoteMediaResolutionError::new("URL source host must not be localhost"));
-    }
-    if let Ok(ip) = host.parse::<IpAddr>() {
-        reject_private_kiro_remote_media_ip(ip)?;
+    // Match the parsed host enum, NOT `host_str()`: the latter returns IPv6
+    // literals bracketed (`[::1]`), which `parse::<IpAddr>()` rejects, letting a
+    // literal-IP URL skip the private-address gate entirely (reqwest dials the
+    // literal without ever calling the filtering resolver).
+    match url
+        .host()
+        .ok_or_else(|| KiroRemoteMediaResolutionError::new("URL source is missing host"))?
+    {
+        url::Host::Domain(domain) => {
+            if domain.eq_ignore_ascii_case("localhost") || domain.ends_with(".localhost") {
+                return Err(KiroRemoteMediaResolutionError::new(
+                    "URL source host must not be localhost",
+                ));
+            }
+        },
+        url::Host::Ipv4(ip) => reject_private_kiro_remote_media_ip(IpAddr::V4(ip))?,
+        url::Host::Ipv6(ip) => reject_private_kiro_remote_media_ip(IpAddr::V6(ip))?,
     }
     Ok(url)
 }
 async fn validate_kiro_remote_media_resolved_addresses(
     url: &url::Url,
 ) -> Result<(), KiroRemoteMediaResolutionError> {
-    let host = url
-        .host_str()
-        .ok_or_else(|| KiroRemoteMediaResolutionError::new("URL source is missing host"))?;
-    if host.parse::<IpAddr>().is_ok() {
-        return Ok(());
-    }
+    // Literal IPs are already vetted by `validate_kiro_remote_media_url` and are
+    // dialed directly (no resolver); only domains need a pre-flight lookup. Use
+    // the host enum so bracketed IPv6 literals aren't misread as domains.
+    let domain = match url
+        .host()
+        .ok_or_else(|| KiroRemoteMediaResolutionError::new("URL source is missing host"))?
+    {
+        url::Host::Domain(domain) => domain.to_string(),
+        url::Host::Ipv4(_) | url::Host::Ipv6(_) => return Ok(()),
+    };
     let port = url
         .port_or_known_default()
         .ok_or_else(|| KiroRemoteMediaResolutionError::new("URL source is missing port"))?;
-    let addresses = tokio::net::lookup_host((host, port)).await.map_err(|err| {
-        KiroRemoteMediaResolutionError::new(format!("failed to resolve URL source host: {err}"))
-    })?;
+    let addresses = tokio::net::lookup_host((domain.as_str(), port))
+        .await
+        .map_err(|err| {
+            KiroRemoteMediaResolutionError::new(format!("failed to resolve URL source host: {err}"))
+        })?;
     let mut resolved_any = false;
     for address in addresses {
         resolved_any = true;
@@ -521,9 +536,164 @@ fn is_private_kiro_remote_media_ipv4(ip: Ipv4Addr) -> bool {
         || ip == Ipv4Addr::UNSPECIFIED
 }
 fn is_private_kiro_remote_media_ipv6(ip: Ipv6Addr) -> bool {
+    // IPv4-mapped (`::ffff:a.b.c.d`) and IPv4-compatible (`::a.b.c.d`) addresses
+    // are dialed as their embedded IPv4 by dual-stack sockets, so an attacker
+    // could smuggle a private IPv4 target (e.g. `::ffff:127.0.0.1`) past the
+    // native-IPv6 checks below. Re-check the embedded IPv4 against the v4 rules.
+    if let Some(mapped) = ip.to_ipv4() {
+        if is_private_kiro_remote_media_ipv4(mapped) {
+            return true;
+        }
+    }
     ip.is_loopback()
         || ip.is_unique_local()
         || ip.is_unicast_link_local()
         || ip.is_unspecified()
         || matches!(ip.segments(), [0x2001, 0x0db8, _, _, _, _, _, _])
 }
+
+/// Drops private/local addresses from a resolved set, rejecting the lookup if
+/// none remain. Used by [`PrivateFilteringDnsResolver`] so the address that is
+/// vetted is exactly the address reqwest dials.
+fn filter_public_kiro_remote_media_addrs(
+    host: &str,
+    addrs: Vec<SocketAddr>,
+) -> Result<Vec<SocketAddr>, String> {
+    let public = addrs
+        .into_iter()
+        .filter(|addr| !is_private_kiro_remote_media_ip(addr.ip()))
+        .collect::<Vec<_>>();
+    if public.is_empty() {
+        return Err(format!(
+            "URL source host `{host}` resolved only to private or local addresses"
+        ));
+    }
+    Ok(public)
+}
+
+/// Custom reqwest DNS resolver for the Kiro remote-media client that filters
+/// out private/local addresses at resolution time.
+///
+/// This is the actual SSRF guard against DNS rebinding: reqwest dials the
+/// addresses this resolver returns, so the IP that is checked is the IP that is
+/// connected to — closing the time-of-check/time-of-use gap that a separate
+/// pre-flight `lookup_host` check cannot (reqwest performs its own resolution
+/// at connect time, which a hostile resolver can answer differently).
+/// IP-literal URLs (never sent to a resolver) remain covered by the literal-IP
+/// gate in [`validate_kiro_remote_media_url`].
+pub(super) struct PrivateFilteringDnsResolver;
+
+impl reqwest::dns::Resolve for PrivateFilteringDnsResolver {
+    fn resolve(&self, name: reqwest::dns::Name) -> reqwest::dns::Resolving {
+        let host = name.as_str().to_string();
+        Box::pin(async move {
+            let resolved = tokio::net::lookup_host((host.as_str(), 0u16))
+                .await
+                .map_err(|err| Box::new(err) as Box<dyn std::error::Error + Send + Sync>)?
+                .collect::<Vec<SocketAddr>>();
+            let public = filter_public_kiro_remote_media_addrs(&host, resolved)
+                .map_err(Box::<dyn std::error::Error + Send + Sync>::from)?;
+            Ok(Box::new(public.into_iter()) as reqwest::dns::Addrs)
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn sa(addr: &str) -> SocketAddr {
+        format!("{addr}:443").parse().expect("valid socket addr")
+    }
+
+    #[test]
+    fn filter_public_addrs_keeps_public_drops_private() {
+        let out = filter_public_kiro_remote_media_addrs("example.test", vec![
+            sa("8.8.8.8"),
+            sa("10.0.0.5"),
+            sa("1.1.1.1"),
+            sa("127.0.0.1"),
+        ])
+        .expect("public addresses remain");
+        assert_eq!(out, vec![sa("8.8.8.8"), sa("1.1.1.1")]);
+    }
+
+    #[test]
+    fn filter_public_addrs_rejects_all_private() {
+        let err = filter_public_kiro_remote_media_addrs("rebind.test", vec![
+            sa("127.0.0.1"),
+            sa("169.254.169.254"),
+            sa("10.1.2.3"),
+        ])
+        .expect_err("an all-private resolution must be rejected");
+        assert!(err.contains("private or local"), "unexpected message: {err}");
+    }
+
+    #[test]
+    fn filter_public_addrs_rejects_empty() {
+        let err = filter_public_kiro_remote_media_addrs("empty.test", Vec::new())
+            .expect_err("an empty resolution must be rejected");
+        assert!(err.contains("private or local"), "unexpected message: {err}");
+    }
+
+    #[tokio::test]
+    async fn resolver_rejects_loopback_hostname() {
+        use std::str::FromStr;
+
+        use reqwest::dns::Resolve;
+        // `localhost` resolves to loopback (RFC 6761), so the resolver must drop
+        // every address and error — proving rebinding to loopback is blocked.
+        let name = reqwest::dns::Name::from_str("localhost").expect("valid dns name");
+        let result = PrivateFilteringDnsResolver.resolve(name).await;
+        assert!(result.is_err(), "localhost must be rejected by the filtering resolver");
+    }
+
+    #[test]
+    fn filter_public_addrs_rejects_ipv4_mapped_private() {
+        // Dual-stack sockets dial `::ffff:a.b.c.d` as the embedded IPv4, so a
+        // hostile resolver could smuggle a private IPv4 target as IPv4-mapped
+        // IPv6. The embedded address must be re-checked against the v4 rules.
+        let err = filter_public_kiro_remote_media_addrs("rebind.test", vec![
+            sa("[::ffff:127.0.0.1]"),
+            sa("[::ffff:10.1.2.3]"),
+            sa("[::ffff:169.254.169.254]"),
+        ])
+        .expect_err("IPv4-mapped private addresses must be rejected");
+        assert!(err.contains("private or local"), "unexpected message: {err}");
+    }
+
+    #[test]
+    fn filter_public_addrs_keeps_ipv4_mapped_public() {
+        let out = filter_public_kiro_remote_media_addrs("example.test", vec![
+            sa("[::ffff:8.8.8.8]"),
+            sa("[::ffff:10.0.0.1]"),
+        ])
+        .expect("the mapped-public address remains");
+        assert_eq!(out, vec![sa("[::ffff:8.8.8.8]")]);
+    }
+
+    #[test]
+    fn validate_url_rejects_literal_loopback_variants() {
+        // host_str() brackets IPv6 literals, so a `parse::<IpAddr>()` gate would
+        // skip them; matching url.host() must reject these (incl. IPv4-mapped and
+        // url-normalized decimal IPv4).
+        for raw in [
+            "http://[::1]/",
+            "http://[::ffff:127.0.0.1]/",
+            "http://[::ffff:10.0.0.1]/",
+            "http://127.0.0.1/",
+            "http://2130706433/",
+        ] {
+            let err = validate_kiro_remote_media_url(raw)
+                .err()
+                .unwrap_or_else(|| panic!("{raw} must be rejected"));
+            assert!(err.to_string().contains("private or local"), "raw={raw} msg={err}");
+        }
+    }
+
+    #[test]
+    fn validate_url_allows_public_literal_ipv6() {
+        validate_kiro_remote_media_url("http://[2606:4700:4700::1111]/")
+            .expect("public literal IPv6 must be allowed");
+    }
+}