From d9753155ae76996834dd370f76d9a28867ed844f Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Fri, 13 Mar 2026 00:40:27 +0000 Subject: [PATCH] fix(security): sanitize reqwest error messages to prevent hostname leakage from_reqwest() was passing raw reqwest error strings through RequestError(err.to_string()), which could contain internal hostnames or URL details. Now classifies errors by kind (redirect, body, decode) with generic messages. Closes #38 --- crates/fetchkit/src/error.rs | 10 +++++- crates/fetchkit/tests/ssrf_security.rs | 45 ++++++++++++++++++++++++++ specs/threat-model.md | 6 ++-- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/crates/fetchkit/src/error.rs b/crates/fetchkit/src/error.rs index 5642004..05e5a7b 100644 --- a/crates/fetchkit/src/error.rs +++ b/crates/fetchkit/src/error.rs @@ -61,13 +61,21 @@ pub enum FetchError { impl FetchError { /// Create an error from a reqwest error + /// + // THREAT[TM-LEAK-001]: Sanitize reqwest errors to avoid leaking internal hostnames/IPs pub fn from_reqwest(err: reqwest::Error) -> Self { if err.is_timeout() { FetchError::FirstByteTimeout } else if err.is_connect() { FetchError::ConnectError(err) + } else if err.is_redirect() { + FetchError::RequestError("redirect error".to_string()) + } else if err.is_body() { + FetchError::RequestError("error reading response body".to_string()) + } else if err.is_decode() { + FetchError::RequestError("error decoding response".to_string()) } else { - FetchError::RequestError(err.to_string()) + FetchError::RequestError("request failed".to_string()) } } } diff --git a/crates/fetchkit/tests/ssrf_security.rs b/crates/fetchkit/tests/ssrf_security.rs index b3383aa..7003e90 100644 --- a/crates/fetchkit/tests/ssrf_security.rs +++ b/crates/fetchkit/tests/ssrf_security.rs @@ -458,3 +458,48 @@ async fn test_input_007_subdomain_not_matched_by_host_prefix() { let result = tool.execute(req).await; assert!(!matches!(result, Err(FetchError::BlockedUrl))); } + +// ============================================================================ +// TM-LEAK-001: Error messages must not leak internal hostnames or IPs +// ============================================================================ + +#[test] +fn test_leak_001_request_error_variants_are_generic() { + // THREAT[TM-LEAK-001]: Verify all RequestError messages are generic and + // don't leak hostnames or IPs from reqwest errors. + // The from_reqwest fallback classifies by error kind, not raw message. + let generic_messages = [ + "redirect error", + "error reading response body", + "error decoding response", + "request failed", + ]; + for msg in &generic_messages { + let err = FetchError::RequestError(msg.to_string()); + let display = err.to_string(); + assert!( + !display.contains("127.0.0.1"), + "Error should not contain IPs: {display}" + ); + assert!( + !display.contains("internal"), + "Error should not contain hostnames: {display}" + ); + } +} + +#[test] +fn test_leak_001_timeout_and_connect_display_are_generic() { + // Verify that timeout and connect error Display messages are static + // strings that don't contain any dynamic content + let timeout_msg = FetchError::FirstByteTimeout.to_string(); + assert_eq!( + timeout_msg, + "Request timed out: server did not respond within 1 second" + ); + + // ConnectError uses "Failed to connect to server" as Display, hiding + // the reqwest source error from the user-facing message + let blocked_msg = FetchError::BlockedUrl.to_string(); + assert_eq!(blocked_msg, "Blocked URL: not allowed by policy"); +} diff --git a/specs/threat-model.md b/specs/threat-model.md index ebe076c..ac1618c 100644 --- a/specs/threat-model.md +++ b/specs/threat-model.md @@ -284,8 +284,10 @@ against unbounded responses (TM-DOS-001). **TM-LEAK-001 — Error message detail (MITIGATED):** FetchKit's error types (`FetchError`) use generic messages that don't include -resolved IP addresses. Connect errors say "Failed to connect to server" without -revealing the specific IP or port that was attempted. +resolved IP addresses or internal hostnames. Connect errors say "Failed to connect +to server" and the `from_reqwest()` fallback path classifies errors by type +(redirect, body, decode) instead of passing through raw reqwest error strings +which could contain hostnames or URL details. ## 6. Content Conversion (TM-CONV)