diff --git a/src/sandbox/proxy/allowlist.rs b/src/sandbox/proxy/allowlist.rs index 207aa272..f3a7bdc6 100644 --- a/src/sandbox/proxy/allowlist.rs +++ b/src/sandbox/proxy/allowlist.rs @@ -144,41 +144,16 @@ impl Default for DomainAllowlist { /// Parse host from a URL string. pub fn extract_host(url: &str) -> Option { - // Determine scheme and extract the rest - let rest = if let Some(stripped) = url.strip_prefix("https://") { - stripped - } else if let Some(stripped) = url.strip_prefix("http://") { - stripped - } else { + let parsed = url::Url::parse(url).ok()?; + if !matches!(parsed.scheme(), "http" | "https") { return None; - }; - - // Find the end of the host (start of path, query, or end of string) - let host_end = rest.find('/').unwrap_or(rest.len()); - let host_and_port = &rest[..host_end]; - - // Remove port if present - let host = if let Some(bracket_idx) = host_and_port.find('[') { - // IPv6 address - let close_bracket = host_and_port.find(']')?; - &host_and_port[bracket_idx + 1..close_bracket] - } else if let Some(colon_idx) = host_and_port.rfind(':') { - // Check if this is a port (all digits after colon) - let after_colon = &host_and_port[colon_idx + 1..]; - if after_colon.chars().all(|c| c.is_ascii_digit()) { - &host_and_port[..colon_idx] - } else { - host_and_port - } - } else { - host_and_port - }; - - if host.is_empty() { - None - } else { - Some(host.to_lowercase()) } + parsed.host_str().map(|h| { + h.strip_prefix('[') + .and_then(|v| v.strip_suffix(']')) + .unwrap_or(h) + .to_lowercase() + }) } #[cfg(test)] @@ -246,6 +221,15 @@ mod tests { extract_host("https://EXAMPLE.COM"), Some("example.com".to_string()) ); + assert_eq!( + extract_host("https://user:pass@api.example.com:443/path"), + Some("api.example.com".to_string()) + ); + assert_eq!( + extract_host("http://[::1]:8080/path"), + Some("::1".to_string()) + ); assert_eq!(extract_host("not-a-url"), None); + assert_eq!(extract_host("ftp://example.com/file"), None); } } diff --git a/src/sandbox/proxy/policy.rs b/src/sandbox/proxy/policy.rs index 2406a694..1c9cad13 100644 --- a/src/sandbox/proxy/policy.rs +++ b/src/sandbox/proxy/policy.rs @@ -24,8 +24,18 @@ pub struct NetworkRequest { impl NetworkRequest { /// Create from a URL string. pub fn from_url(method: &str, url: &str) -> Option { - let host = crate::sandbox::proxy::allowlist::extract_host(url)?; - let path = extract_path(url); + let parsed = url::Url::parse(url).ok()?; + if !matches!(parsed.scheme(), "http" | "https") { + return None; + } + + let host = parsed.host_str()?; + let host = host + .strip_prefix('[') + .and_then(|v| v.strip_suffix(']')) + .unwrap_or(host) + .to_lowercase(); + let path = parsed.path().to_string(); Some(Self { method: method.to_uppercase(), @@ -37,15 +47,15 @@ impl NetworkRequest { } /// Extract path from a URL. +#[cfg(test)] fn extract_path(url: &str) -> String { - // Find the start of the path (after ://) - if let Some(idx) = url.find("://") { - let rest = &url[idx + 3..]; - if let Some(path_start) = rest.find('/') { - return rest[path_start..].to_string(); - } + let Ok(parsed) = url::Url::parse(url) else { + return "/".to_string(); + }; + if !matches!(parsed.scheme(), "http" | "https") { + return "/".to_string(); } - "/".to_string() + parsed.path().to_string() } /// Decision for a network request. @@ -203,6 +213,11 @@ mod tests { ); assert_eq!(extract_path("https://example.com"), "/".to_string()); assert_eq!(extract_path("https://example.com/"), "/".to_string()); + assert_eq!( + extract_path("https://example.com/path?q=1#frag"), + "/path".to_string() + ); + assert_eq!(extract_path("ftp://example.com/path"), "/".to_string()); } #[tokio::test] diff --git a/src/tools/wasm/allowlist.rs b/src/tools/wasm/allowlist.rs index d27a5037..36b5dac6 100644 --- a/src/tools/wasm/allowlist.rs +++ b/src/tools/wasm/allowlist.rs @@ -170,74 +170,89 @@ struct ParsedUrl { path: String, } -/// Simple URL parser (avoids pulling in a full URL crate). +/// Parse and normalize URL components for allowlist matching. fn parse_url(url: &str) -> Result { - // Find scheme - let (scheme, rest) = url - .split_once("://") - .ok_or_else(|| "Missing scheme (expected http:// or https://)".to_string())?; - - let scheme = scheme.to_lowercase(); + let parsed = url::Url::parse(url).map_err(|e| format!("URL parse failed: {e}"))?; + let scheme = parsed.scheme().to_lowercase(); if scheme != "http" && scheme != "https" { return Err(format!("Unsupported scheme: {}", scheme)); } - // Reject URLs with userinfo (user:pass@host) to prevent allowlist bypass. - // A URL like https://api.openai.com@evil.com/ would match the allowlist - // for api.openai.com but actually send traffic to evil.com. - let authority = match rest.find('/') { - Some(idx) => &rest[..idx], - None => rest, - }; - if authority.contains('@') { + // Reject URLs with userinfo (user:pass@host) to prevent host-confusion bypasses. + if !parsed.username().is_empty() || parsed.password().is_some() { return Err("URL contains userinfo (@) which is not allowed".to_string()); } - // Split host from path - let (host_and_port, path) = match rest.find('/') { - Some(idx) => (&rest[..idx], &rest[idx..]), - None => (rest, "/"), - }; - - // Remove port from host - let host = match host_and_port.rfind(':') { - Some(idx) => { - // Make sure this isn't an IPv6 address - if host_and_port.starts_with('[') { - // IPv6: [::1]:8080 or [::1] - if let Some(bracket_idx) = host_and_port.find(']') { - // Extract the IPv6 address without brackets - &host_and_port[1..bracket_idx] - } else { - return Err("Invalid IPv6 address".to_string()); - } - } else { - &host_and_port[..idx] - } - } - None => host_and_port, - }; - - // Reject URLs with userinfo (user:pass@host). - // A URL like https://api.openai.com@evil.com/ confuses the parser into - // seeing "api.openai.com" as the host, but reqwest actually sends to - // "evil.com". Block any '@' in the authority section to prevent this. - if host.contains('@') || host_and_port.contains('@') { - return Err("URL contains userinfo (@) which is not allowed".to_string()); - } - - // Validate host - if host.is_empty() { - return Err("Empty host".to_string()); - } + let host = parsed.host_str().ok_or_else(|| "Empty host".to_string())?; + let host = host + .strip_prefix('[') + .and_then(|h| h.strip_suffix(']')) + .unwrap_or(host) + .to_lowercase(); + let normalized_path = normalize_path(parsed.path())?; Ok(ParsedUrl { scheme, - host: host.to_lowercase(), - path: path.to_string(), + host, + path: normalized_path, }) } +fn normalize_path(path: &str) -> Result { + let mut segments: Vec = Vec::new(); + for raw_segment in path.split('/') { + if !has_valid_percent_encoding(raw_segment) { + return Err(format!( + "Invalid percent-encoding in path segment: {raw_segment}" + )); + } + + let segment = urlencoding::decode(raw_segment) + .map_err(|_| format!("Invalid percent-encoding in path segment: {raw_segment}"))?; + let segment = segment.as_ref(); + + // Encoded separators introduce ambiguous semantics across downstream handlers. + if segment.contains('/') || segment.contains('\\') { + return Err("Path segment contains encoded path separator".to_string()); + } + + match segment { + "" | "." => {} + ".." => { + segments.pop(); + } + _ => segments.push(segment.to_string()), + } + } + + let mut result = String::with_capacity(path.len().max(1)); + result.push('/'); + result.push_str(&segments.join("/")); + if path.len() > 1 && path.ends_with('/') && !result.ends_with('/') { + result.push('/'); + } + Ok(result) +} + +fn has_valid_percent_encoding(segment: &str) -> bool { + let bytes = segment.as_bytes(); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'%' { + if i + 2 >= bytes.len() + || !bytes[i + 1].is_ascii_hexdigit() + || !bytes[i + 2].is_ascii_hexdigit() + { + return false; + } + i += 3; + } else { + i += 1; + } + } + true +} + #[cfg(test)] mod tests { use crate::tools::wasm::allowlist::{AllowlistValidator, DenyReason}; @@ -380,6 +395,68 @@ mod tests { } } + #[test] + fn test_path_traversal_blocked() { + let validator = validator_with_patterns(); + assert!(!validator.validate("https://api.openai.com/v1/../admin", "GET").is_allowed()); + assert!(!validator.validate("https://api.openai.com/v1/../../etc/passwd", "GET").is_allowed()); + assert!(!validator.validate("https://api.openai.com/v1/%2E%2E/admin", "GET").is_allowed()); + assert!(!validator.validate("https://api.openai.com/v1/%2e%2e/%2e%2e/root", "GET").is_allowed()); + assert!(validator.validate("https://api.openai.com/v1/chat/completions", "POST").is_allowed()); + } + + #[test] + fn test_normalize_path() { + use super::normalize_path; + assert_eq!(normalize_path("/v1/../admin").unwrap(), "/admin"); + assert_eq!( + normalize_path("/v1/chat/completions").unwrap(), + "/v1/chat/completions" + ); + assert_eq!(normalize_path("/v1/./chat").unwrap(), "/v1/chat"); + assert_eq!( + normalize_path("/v1/../../../etc/passwd").unwrap(), + "/etc/passwd" + ); + assert_eq!(normalize_path("/v1/%2e%2e/admin").unwrap(), "/admin"); + assert_eq!(normalize_path("/").unwrap(), "/"); + assert_eq!(normalize_path("/v1/").unwrap(), "/v1/"); + } + + #[test] + fn test_invalid_encoded_path_rejected() { + let validator = validator_with_patterns(); + let result = validator.validate("https://api.openai.com/v1/%ZZ/chat", "GET"); + assert!(!result.is_allowed()); + if let super::AllowlistResult::Denied(reason) = result { + assert!(matches!(reason, DenyReason::InvalidUrl(_))); + } else { + panic!("Expected denied"); + } + } + + #[test] + fn test_encoded_separator_rejected() { + let validator = validator_with_patterns(); + let result = validator.validate("https://api.openai.com/v1/%2Fadmin", "GET"); + assert!(!result.is_allowed()); + if let super::AllowlistResult::Denied(reason) = result { + assert!(matches!(reason, DenyReason::InvalidUrl(_))); + } else { + panic!("Expected denied"); + } + } + + #[test] + fn test_percent_encoding_validator() { + use super::has_valid_percent_encoding; + assert!(has_valid_percent_encoding("%2F")); + assert!(has_valid_percent_encoding("hello%20world")); + assert!(!has_valid_percent_encoding("%")); + assert!(!has_valid_percent_encoding("%2")); + assert!(!has_valid_percent_encoding("%ZZ")); + } + #[test] fn test_url_with_port() { let validator = diff --git a/src/tools/wasm/wrapper.rs b/src/tools/wasm/wrapper.rs index 72e31d23..389d090b 100644 --- a/src/tools/wasm/wrapper.rs +++ b/src/tools/wasm/wrapper.rs @@ -992,51 +992,37 @@ async fn resolve_host_credentials( /// Also handles IPv6 bracket notation like `http://[::1]:8080/path`. /// Returns None for malformed URLs. fn extract_host_from_url(url: &str) -> Option { - let after_scheme = url - .strip_prefix("https://") - .or_else(|| url.strip_prefix("http://"))?; - let end = after_scheme - .find(['/', '?', '#']) - .unwrap_or(after_scheme.len()); - let host_port = &after_scheme[..end]; - // Strip userinfo (user:pass@host) - let after_userinfo = host_port - .rfind('@') - .map(|i| &host_port[i + 1..]) - .unwrap_or(host_port); - // Handle IPv6 bracket notation: [::1]:port -> ::1 - if after_userinfo.starts_with('[') { - let closing = after_userinfo.find(']')?; - return Some(after_userinfo[1..closing].to_string()); + let parsed = url::Url::parse(url).ok()?; + if !matches!(parsed.scheme(), "http" | "https") { + return None; } - // Regular host:port -> host - let host = after_userinfo - .rfind(':') - .map(|i| &after_userinfo[..i]) - .unwrap_or(after_userinfo); - Some(host.to_string()) + parsed.host_str().map(|h| { + h.strip_prefix('[') + .and_then(|v| v.strip_suffix(']')) + .unwrap_or(h) + .to_lowercase() + }) } /// Resolve the URL's hostname and reject connections to private/internal IP addresses. /// This prevents DNS rebinding attacks where an attacker's domain resolves to an /// internal IP after passing the allowlist check. fn reject_private_ip(url: &str) -> Result<(), String> { - let host = url - .split("://") - .nth(1) - .and_then(|rest| { - let host_and_port = rest.split('/').next().unwrap_or(rest); - // Strip port - if host_and_port.starts_with('[') { - // IPv6 - host_and_port.find(']').map(|i| &host_and_port[1..i]) - } else { - Some( - host_and_port - .rfind(':') - .map_or(host_and_port, |i| &host_and_port[..i]), - ) - } + let parsed = url::Url::parse(url) + .map_err(|e| format!("Failed to parse URL: {e}"))?; + if !matches!(parsed.scheme(), "http" | "https") { + return Err(format!("Unsupported URL scheme: {}", parsed.scheme())); + } + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err("URL contains userinfo (@) which is not allowed".to_string()); + } + + let host = parsed + .host_str() + .map(|h| { + h.strip_prefix('[') + .and_then(|v| v.strip_suffix(']')) + .unwrap_or(h) }) .ok_or_else(|| "Failed to parse host from URL".to_string())?;