From 1d4a6b304a34b8263a3fc1856dda870dc021d96a Mon Sep 17 00:00:00 2001 From: LB7666 Date: Mon, 1 Jun 2026 04:15:54 +0800 Subject: [PATCH 1/2] fix(llm-access): block DNS-rebinding SSRF in kiro remote-media fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The kiro remote-media fetcher validated resolved IPs with its own `tokio::net::lookup_host` (rejecting private/local addresses), then let reqwest perform an INDEPENDENT DNS resolution at connect time. A hostile authoritative DNS server could answer the pre-flight check with a public IP and the connect-time resolution with 127.0.0.1 / 169.254.169.254 / RFC1918, bypassing the guard — a TOCTOU DNS-rebinding SSRF (reachable when a key has kiro_remote_media_resolution_enabled, since the source URLs come from the request payload). Found in PR #20 review (gemini). Fix: a custom `reqwest::dns::Resolve` (PrivateFilteringDnsResolver) installed on KIRO_REMOTE_MEDIA_CLIENT via `dns_resolver`. It resolves the host and filters out private/local addresses, returning only public ones (erroring if none remain). Because reqwest dials exactly the addresses the resolver returns, the IP that is vetted IS the IP connected to — closing the check/use gap. IP-literal URLs (never sent to a resolver) remain covered by the existing literal-IP gate in validate_kiro_remote_media_url; the strict pre-flight check is kept for early, clear errors (defense in depth). Tests: pure `filter_public_kiro_remote_media_addrs` cases (keeps public / drops private / rejects all-private / rejects empty) + a resolver test that `localhost` (RFC 6761 loopback) is rejected end-to-end. Verification: - cargo clippy -p llm-access --all-targets -- -D warnings -> clean - cargo test -p llm-access provider:: -> 115 passed (111 prior + 4 new) - rustfmt on the 2 changed files only; deps/lance, deps/lancedb untouched Co-Authored-By: Claude Opus 4.8 --- llm-access/src/provider.rs | 1 + llm-access/src/provider/kiro_media.rs | 99 ++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/llm-access/src/provider.rs b/llm-access/src/provider.rs index 48bdbfc..6992b17 100644 --- a/llm-access/src/provider.rs +++ b/llm-access/src/provider.rs @@ -496,6 +496,7 @@ static KIRO_REMOTE_MEDIA_CLIENT: std::sync::LazyLock = 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)) diff --git a/llm-access/src/provider/kiro_media.rs b/llm-access/src/provider/kiro_media.rs index caacde3..1fdd8ee 100644 --- a/llm-access/src/provider/kiro_media.rs +++ b/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 _; @@ -527,3 +527,100 @@ fn is_private_kiro_remote_media_ipv6(ip: Ipv6Addr) -> bool { || 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, +) -> Result, String> { + let public = addrs + .into_iter() + .filter(|addr| !is_private_kiro_remote_media_ip(addr.ip())) + .collect::>(); + 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)? + .collect::>(); + let public = filter_public_kiro_remote_media_addrs(&host, resolved) + .map_err(Box::::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"); + } +} From e182cc4571db2ec42194905c32968f9e94095aa3 Mon Sep 17 00:00:00 2001 From: LB7666 Date: Mon, 1 Jun 2026 13:10:24 +0800 Subject: [PATCH 2/2] fix(llm-access): close IPv4-mapped and literal-IPv6 SSRF gate holes CR on PR #21 surfaced two residual bypasses of the kiro remote-media private-address gate: - IPv4-mapped (`::ffff:a.b.c.d`) / IPv4-compatible (`::a.b.c.d`) IPv6 addresses are dialed as their embedded IPv4 by dual-stack sockets, so e.g. `::ffff:127.0.0.1` slipped past the native-IPv6 checks. Re-check the embedded IPv4 (via `Ipv6Addr::to_ipv4`) against the v4 rules in both the literal gate and the resolving filter. - `url.host_str()` brackets IPv6 literals (`[::1]`), so the `parse::()` literal gate never matched them and a literal-IP URL skipped the private check entirely (reqwest dials the literal without ever consulting the filtering resolver). Match the parsed `url.host()` enum (Domain/Ipv4/Ipv6) instead, in both the pre-flight validate and the resolve-addresses pre-check. Tests: IPv4-mapped private rejection + mapped-public retention; literal loopback / mapped-private / url-normalized decimal-IPv4 URL rejection; public literal IPv6 allowed. Co-Authored-By: Claude Opus 4.8 --- llm-access/src/provider/kiro_media.rs | 107 ++++++++++++++++++++++---- 1 file changed, 90 insertions(+), 17 deletions(-) diff --git a/llm-access/src/provider/kiro_media.rs b/llm-access/src/provider/kiro_media.rs index 1fdd8ee..cde77da 100644 --- a/llm-access/src/provider/kiro_media.rs +++ b/llm-access/src/provider/kiro_media.rs @@ -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::() { - reject_private_kiro_remote_media_ip(ip)?; + // Match the parsed host enum, NOT `host_str()`: the latter returns IPv6 + // literals bracketed (`[::1]`), which `parse::()` 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::().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,6 +536,15 @@ 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() @@ -623,4 +647,53 @@ mod tests { 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::()` 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"); + } }