updated in response to ossf scorecard

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`.

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,

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`
 

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::<std::net::IpAddr>() {
+                // 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()
     })
 }