From 8ec38dece6f16146ab154e4dd35722f5900741f7 Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Tue, 24 Mar 2026 15:33:00 -0700 Subject: [PATCH] fix(sandbox): block unspecified IPs in SSRF checks --- crates/openshell-sandbox/src/proxy.rs | 57 ++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index a9e377f5..1f38a2cc 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -1210,18 +1210,20 @@ fn query_tls_mode( } } -/// Check if an IP address is internal (loopback, private RFC1918, or link-local). +/// Check if an IP address is internal (loopback, private RFC1918, link-local, or unspecified). /// /// This is a defense-in-depth check to prevent SSRF via the CONNECT proxy. /// It covers: -/// - IPv4 loopback (127.0.0.0/8), private (10/8, 172.16/12, 192.168/16), link-local (169.254/16) -/// - IPv6 loopback (`::1`), link-local (`fe80::/10`), ULA (`fc00::/7`) +/// - IPv4 loopback (127.0.0.0/8), private (10/8, 172.16/12, 192.168/16), link-local (169.254/16), unspecified (`0.0.0.0`) +/// - IPv6 loopback (`::1`), link-local (`fe80::/10`), ULA (`fc00::/7`), unspecified (`::`) /// - IPv4-mapped IPv6 addresses (`::ffff:x.x.x.x`) are unwrapped and checked as IPv4 fn is_internal_ip(ip: IpAddr) -> bool { match ip { - IpAddr::V4(v4) => v4.is_loopback() || v4.is_private() || v4.is_link_local(), + IpAddr::V4(v4) => { + v4.is_loopback() || v4.is_private() || v4.is_link_local() || v4.is_unspecified() + } IpAddr::V6(v6) => { - if v6.is_loopback() { + if v6.is_loopback() || v6.is_unspecified() { return true; } // fe80::/10 — IPv6 link-local @@ -1234,7 +1236,10 @@ fn is_internal_ip(ip: IpAddr) -> bool { } // Check IPv4-mapped IPv6 (::ffff:x.x.x.x) if let Some(v4) = v6.to_ipv4_mapped() { - return v4.is_loopback() || v4.is_private() || v4.is_link_local(); + return v4.is_loopback() + || v4.is_private() + || v4.is_link_local() + || v4.is_unspecified(); } false } @@ -1287,14 +1292,14 @@ async fn resolve_and_reject_internal( /// Check if an IP address is always blocked regardless of policy. /// -/// Loopback and link-local addresses are never allowed even when an endpoint +/// Loopback, link-local, and unspecified addresses are never allowed even when an endpoint /// has `allowed_ips` configured. This prevents proxy bypass (loopback) and /// cloud metadata SSRF (link-local 169.254.x.x). fn is_always_blocked_ip(ip: IpAddr) -> bool { match ip { - IpAddr::V4(v4) => v4.is_loopback() || v4.is_link_local(), + IpAddr::V4(v4) => v4.is_loopback() || v4.is_link_local() || v4.is_unspecified(), IpAddr::V6(v6) => { - if v6.is_loopback() { + if v6.is_loopback() || v6.is_unspecified() { return true; } // fe80::/10 — IPv6 link-local @@ -1303,7 +1308,7 @@ fn is_always_blocked_ip(ip: IpAddr) -> bool { } // Check IPv4-mapped IPv6 (::ffff:x.x.x.x) if let Some(v4) = v6.to_ipv4_mapped() { - return v4.is_loopback() || v4.is_link_local(); + return v4.is_loopback() || v4.is_link_local() || v4.is_unspecified(); } false } @@ -2023,6 +2028,11 @@ mod tests { assert!(is_internal_ip(IpAddr::V4(Ipv4Addr::new(169, 254, 0, 1)))); } + #[test] + fn test_rejects_ipv4_unspecified() { + assert!(is_internal_ip(IpAddr::V4(Ipv4Addr::UNSPECIFIED))); + } + #[test] fn test_allows_ipv4_public() { assert!(!is_internal_ip(IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)))); @@ -2043,6 +2053,11 @@ mod tests { assert!(is_internal_ip(IpAddr::V6(Ipv6Addr::LOCALHOST))); } + #[test] + fn test_rejects_ipv6_unspecified() { + assert!(is_internal_ip(IpAddr::V6(Ipv6Addr::UNSPECIFIED))); + } + #[test] fn test_rejects_ipv6_link_local() { // fe80::1 @@ -2325,6 +2340,16 @@ mod tests { )))); } + #[test] + fn test_always_blocked_ipv4_unspecified() { + assert!(is_always_blocked_ip(IpAddr::V4(Ipv4Addr::UNSPECIFIED))); + } + + #[test] + fn test_always_blocked_ipv6_unspecified() { + assert!(is_always_blocked_ip(IpAddr::V6(Ipv6Addr::UNSPECIFIED))); + } + #[test] fn test_always_blocked_ipv4_mapped_v6_loopback() { let v6 = Ipv4Addr::LOCALHOST.to_ipv6_mapped(); @@ -2430,6 +2455,18 @@ mod tests { ); } + #[tokio::test] + async fn test_resolve_check_allowed_ips_blocks_unspecified() { + let nets = parse_allowed_ips(&["0.0.0.0/0".to_string()]).unwrap(); + let result = resolve_and_check_allowed_ips("0.0.0.0", 80, &nets).await; + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!( + err.contains("always-blocked"), + "expected 'always-blocked' in error: {err}" + ); + } + #[tokio::test] async fn test_resolve_check_allowed_ips_rejects_outside_allowlist() { // 8.8.8.8 resolves to a public IP which is NOT in 10.0.0.0/8