diff --git a/CHANGELOG.md b/CHANGELOG.md index 27be9a9..7b4462f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ All notable changes to this project will be documented in this file. ## [v1.91.0] -- Added SSRF protection for credential validation: outbound HTTP requests now block connections to loopback, private, link-local, and other non-public IP addresses. Automatic HTTP redirects are blocked during validation to prevent redirect-based SSRF. Use `--allow-internal-ips` to opt out when scanning internal infrastructure. +- Added SSRF protection for credential validation: outbound HTTP requests now block connections to loopback, private, link-local, and other non-public IP addresses. HTTP redirect targets are DNS-resolved and validated against the same SSRF rules. Use `--allow-internal-ips` to opt out when scanning internal infrastructure. - Consolidated JWT SSRF checks to use the shared `is_ssrf_safe_ip` function, covering additional reserved ranges (CGNAT, documentation, benchmarking, IPv6 unique-local). - Removed `ipnet` dependency from `kingfisher-scanner` (no longer needed). - Remediated current RustSec vulnerability findings by upgrading core dependencies including `gix`, `mysql_async`, `axum`, `indicatif`, `quick-xml`, and `console`. diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index 2e24bf7..5638907 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -484,8 +484,9 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { /// DNS resolution when connecting. A DNS-rebinding attack could theoretically /// return a public IP for this check and a private IP for the actual connection. /// Fully eliminating this TOCTOU gap would require a custom resolver/connector -/// that pins resolved IPs; automatic redirect blocking (see -/// `ssrf_safe_redirect_policy`) mitigates the most practical exploitation path. +/// that pins resolved IPs. In practice, callers should also disable automatic +/// redirects or use a redirect-blocking policy on the HTTP client to mitigate +/// the most practical exploitation paths. pub async fn check_url_resolvable( url: &Url, allow_internal_ips: bool, diff --git a/docs/USAGE.md b/docs/USAGE.md index 9d69ebc..656c405 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1004,11 +1004,12 @@ By default, validation requests are rejected if the target hostname resolves to | `fc00::/7` | IPv6 unique-local | | `2001:db8::/32` | IPv6 documentation (RFC 3849) | | `::ffff:0:0/96` | IPv4-mapped IPv6 (checked against IPv4 rules) | +| `::/96` | IPv4-compatible IPv6 (deprecated) | | `240.0.0.0/4` | Reserved for future use (includes broadcast) | | `fec0::/10` | IPv6 site-local (deprecated, RFC 3879) | | Multicast, benchmarking ranges | Other reserved ranges | -Automatic HTTP redirects are blocked during credential validation to prevent redirect-based SSRF bypasses. +HTTP redirects during credential validation are also validated: each redirect target is resolved via DNS and checked against the same SSRF rules above. Redirects to non-public IPs are blocked. When `--allow-internal-ips` is used, redirect validation is disabled along with all other SSRF protections. ### `--allow-internal-ips` diff --git a/src/validation.rs b/src/validation.rs index 44779c7..5ad795f 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -122,22 +122,54 @@ pub struct ValidationClients { pub allow_internal_ips: bool, } -/// Build a redirect policy that prevents SSRF via HTTP redirects. +/// Build a redirect policy that validates redirect targets against SSRF rules. /// -/// Automatic redirects are blocked entirely when SSRF protection is enabled. -/// An attacker could supply a URL that initially points to a public endpoint -/// but redirects to an internal/private address (including via a hostname that -/// resolves to a private IP). Because reqwest's redirect callback is synchronous -/// and cannot perform async DNS resolution, selectively filtering redirect -/// targets is insufficient. Blocking all redirects is the safe default; -/// credential validation endpoints rarely require redirect-following. +/// Each redirect hop is checked: IP-literal targets are validated directly via +/// `is_ssrf_safe_ip`, and hostname targets are resolved synchronously via +/// `std::net::ToSocketAddrs` so that all resolved IPs can be checked. This +/// closes the hostname-redirect SSRF gap (e.g., a public URL that 302s to an +/// attacker-controlled hostname resolving to `169.254.169.254`). +/// +/// **Note:** Using blocking DNS in the redirect callback is acceptable because +/// reqwest runs redirect callbacks on its connection thread, not the tokio +/// event loop, and the DNS latency is negligible relative to the HTTP request. pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { reqwest::redirect::Policy::custom(|attempt| { + // Cap redirect depth (reqwest default is 10) + if attempt.previous().len() >= 10 { + return attempt.error("too many redirects"); + } + // Extract URL info before potentially moving `attempt`. let url = attempt.url().clone(); - attempt.error(format!( - "SSRF protection: redirect to {} blocked (automatic redirects disabled)", - url - )) + if let Some(host) = url.host_str() { + if let Ok(ip) = host.parse::() { + // IP-literal: check directly without DNS. + if !kingfisher_scanner::validation::is_ssrf_safe_ip(&ip) { + return attempt.error(format!( + "SSRF protection: redirect to non-public IP {} blocked", + ip + )); + } + } else { + // Hostname: resolve synchronously and check all resolved IPs. + let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); + if let Ok(addrs) = std::net::ToSocketAddrs::to_socket_addrs(&(host, port as u16)) { + for addr in addrs { + if !kingfisher_scanner::validation::is_ssrf_safe_ip(&addr.ip()) { + return attempt.error(format!( + "SSRF protection: redirect to '{}' resolves to non-public IP {} — blocked", + host, + addr.ip() + )); + } + } + } + // If DNS resolution fails, allow the redirect — reqwest will + // fail on connect anyway, and we don't want to silently block + // valid hostnames that are transiently unresolvable. + } + } + attempt.follow() }) }