diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index f5af606..07080c9 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -480,6 +480,21 @@ pub async fn check_url_resolvable( allow_internal_ips: bool, ) -> Result<(), Box> { let host = url.host_str().ok_or("No host in URL")?; + + // If the host is already an IP literal, check it directly without DNS. + if let Ok(ip) = host.parse::() { + if !allow_internal_ips && !is_ssrf_safe_ip(&ip) { + return Err(format!( + "SSRF protection: resolved IP {} for host '{}' is not a public address. \ + Use --allow-internal-ips to permit internal addresses.", + ip, host + ) + .into()); + } + return Ok(()); + } + + // Hostname — resolve via DNS and check each resolved address. let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); let addr = format!("{}:{}", host, port); let mut resolved_any = false; @@ -675,4 +690,14 @@ mod tests { let result = check_url_resolvable(&url, true).await; assert!(result.is_ok(), "expected Ok with allow_internal_ips=true, got: {:?}", result); } + + #[tokio::test] + async fn check_url_resolvable_rejects_ipv6_loopback_literal() { + // IPv6 literal URL — brackets are handled by reqwest::Url, host_str() returns "::1" + let url = Url::parse("https://[::1]/test").unwrap(); + let result = check_url_resolvable(&url, false).await; + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!(err.contains("SSRF protection"), "expected SSRF error, got: {}", err); + } } diff --git a/docs/USAGE.md b/docs/USAGE.md index 72bcc55..bf2f6fd 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1016,7 +1016,7 @@ If you are scanning infrastructure that uses internal endpoints for credential v # Scan with SSRF protection disabled (allows requests to internal IPs) kingfisher scan --allow-internal-ips ./repo -# Also works with validate and revoke commands +# Also works with the validate command kingfisher validate --allow-internal-ips --rule kingfisher.artifactory.1 ``` diff --git a/src/direct_validate.rs b/src/direct_validate.rs index 87caf06..e35df49 100644 --- a/src/direct_validate.rs +++ b/src/direct_validate.rs @@ -296,10 +296,16 @@ async fn execute_http_validation( parser: &liquid::Parser, timeout: Duration, retries: u32, + allow_internal_ips: bool, ) -> Result { // Render the URL let url = render_and_parse_url(parser, globals, &http_validation.request.url).await?; + // SSRF check: verify the resolved IP is public before making the request + crate::validation::utils::check_url_resolvable(&url, allow_internal_ips) + .await + .map_err(|e| anyhow!("URL resolution failed: {}", e))?; + debug!("Validating against URL: {}", url); // Build the request @@ -438,11 +444,16 @@ pub async fn run_direct_validation( crate::cli::global::TlsMode::Off | crate::cli::global::TlsMode::Lax ); - // Build HTTP client + // Build HTTP client with SSRF-safe redirect policy when applicable let client = Client::builder() .danger_accept_invalid_certs(use_lax_tls) .timeout(Duration::from_secs(args.timeout)) .user_agent(GLOBAL_USER_AGENT.as_str()) + .redirect(if global_args.allow_internal_ips { + reqwest::redirect::Policy::default() + } else { + crate::validation::ssrf_safe_redirect_policy() + }) .gzip(true) .deflate(true) .brotli(true) @@ -569,6 +580,7 @@ pub async fn run_direct_validation( &parser, timeout, args.retries, + global_args.allow_internal_ips, ) .await? } diff --git a/src/validation.rs b/src/validation.rs index d9e2f72..58758d0 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -133,7 +133,7 @@ pub struct ValidationClients { /// internal IP would bypass this check. Fully closing this gap requires /// disabling automatic redirects and manually following them with async DNS /// validation at each hop — a future enhancement. -fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { +pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { reqwest::redirect::Policy::custom(|attempt| { if let Some(host) = attempt.url().host_str() { // For IP-literal hosts, check directly without DNS.