forked from mirrors/kingfisher
updated in response to ossf scorecard
This commit is contained in:
parent
9c8c63db90
commit
4e9a7364cd
4 changed files with 40 additions and 3 deletions
|
|
@ -480,6 +480,21 @@ pub async fn check_url_resolvable(
|
|||
allow_internal_ips: bool,
|
||||
) -> Result<(), Box<dyn std::error::Error>> {
|
||||
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::<std::net::IpAddr>() {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
```
|
||||
|
||||
|
|
|
|||
|
|
@ -296,10 +296,16 @@ async fn execute_http_validation(
|
|||
parser: &liquid::Parser,
|
||||
timeout: Duration,
|
||||
retries: u32,
|
||||
allow_internal_ips: bool,
|
||||
) -> Result<DirectValidationResult> {
|
||||
// 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?
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue