diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 39b07ef..91eb686 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.124.14" +version = "0.124.15" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 1c5331e..cf38a5d 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.124.14" +version = "0.124.15" edition = "2021" default-run = "mangalord" diff --git a/backend/src/app.rs b/backend/src/app.rs index 6288b49..f3831aa 100644 --- a/backend/src/app.rs +++ b/backend/src/app.rs @@ -605,16 +605,16 @@ async fn spawn_crawler_daemon( if let Some(proxy) = &cfg.proxy { http_builder = http_builder .proxy(reqwest::Proxy::all(proxy).with_context(|| format!("parse proxy: {proxy}"))?); - } else { - // DNS-rebinding guard: reject hosts that resolve to a private/internal - // IP, complementing the string-level allowlist check which can't see - // post-resolution addresses. ONLY on the direct (unproxied) path. With - // a `socks5h://` proxy the target hostname is resolved by the proxy — - // reqwest never resolves it — so the only name this resolver would ever - // see is the proxy's OWN host, which legitimately lives on a private - // Docker IP (e.g. `tor` → 172.x). Attaching it there rejected every - // fetch ("SOCKS error: failed to create underlying connection") for - // zero security gain, since Tor can't route to internal ranges anyway. + } + // DNS-rebinding guard: reject hosts that resolve to a private/internal IP, + // complementing the string-level allowlist check which can't see + // post-resolution addresses. Attached on the direct path AND on http(s) + // proxies (reqwest resolves the target itself there). Skipped only for SOCKS + // proxies, where the proxy — not reqwest — resolves the target, so the only + // name this resolver would see is the proxy's OWN host (legitimately on a + // private Docker IP, e.g. `tor` → 172.x); attaching it there rejected every + // fetch for zero gain. See `should_attach_safe_resolver`. + if crate::crawler::safety::should_attach_safe_resolver(cfg.proxy.as_deref()) { http_builder = http_builder.dns_resolver(crate::crawler::safety::safe_dns_resolver()); } let http = http_builder.build().context("build crawler reqwest")?; diff --git a/backend/src/crawler/safety.rs b/backend/src/crawler/safety.rs index 1ada91b..436bb30 100644 --- a/backend/src/crawler/safety.rs +++ b/backend/src/crawler/safety.rs @@ -289,6 +289,40 @@ pub fn safe_dns_resolver() -> Arc { Arc::new(SafeResolver) } +/// Whether the DNS-rebinding [`SafeResolver`] should be attached to a crawler +/// reqwest client given the configured proxy (if any). +/// +/// The resolver only guards targets that *reqwest itself* resolves: +/// - **No proxy** (direct): reqwest resolves the target — attach. +/// - **`http(s)://` proxy**: reqwest still resolves the target host locally and +/// issues a `CONNECT`, so a hostname resolving to a private IP must be +/// refused — attach. +/// - **`socks5://` / `socks5h://` / `socks4://` (Tor)**: the *proxy* resolves +/// the target; reqwest only ever resolves the proxy's own host, which +/// legitimately lives on a private Docker IP (e.g. `tor` → 172.x). Attaching +/// the resolver there rejects every fetch for zero security gain (a SOCKS +/// proxy can't route into internal ranges anyway) — do **not** attach. +/// +/// This narrows commit 134ab54, which dropped the resolver for *any* proxy, back +/// to SOCKS-only so the http(s)-proxy path keeps its DNS-rebinding guard. +pub fn should_attach_safe_resolver(proxy: Option<&str>) -> bool { + match proxy { + None => true, + Some(p) => !is_socks_proxy(p), + } +} + +/// True when `proxy`'s scheme is a SOCKS variant (`socks4`, `socks5`, +/// `socks5h`). A scheme-less value (reqwest treats it as HTTP) is not SOCKS. +fn is_socks_proxy(proxy: &str) -> bool { + proxy + .split_once("://") + .map(|(scheme, _)| scheme) + .unwrap_or("") + .to_ascii_lowercase() + .starts_with("socks") +} + #[derive(Debug, thiserror::Error, PartialEq, Eq)] pub enum UrlSafetyError { #[error("URL is not parseable")] @@ -743,6 +777,24 @@ mod tests { assert!(err.to_string().contains("rebind.attacker")); } + #[test] + fn safe_resolver_attaches_except_for_socks_proxies() { + // Direct + http(s) proxies: reqwest resolves the target, so the + // DNS-rebinding guard must be attached. + assert!(should_attach_safe_resolver(None)); + assert!(should_attach_safe_resolver(Some("http://proxy.internal:8080"))); + assert!(should_attach_safe_resolver(Some("https://proxy.internal:8080"))); + assert!(should_attach_safe_resolver(Some("HTTP://Proxy:8080"))); + // A scheme-less proxy is treated as HTTP by reqwest — keep the guard. + assert!(should_attach_safe_resolver(Some("proxy.internal:8080"))); + // SOCKS variants (incl. Tor): the proxy resolves, so attaching the + // resolver would reject every fetch for zero gain. + assert!(!should_attach_safe_resolver(Some("socks5://tor:9050"))); + assert!(!should_attach_safe_resolver(Some("socks5h://tor:9050"))); + assert!(!should_attach_safe_resolver(Some("socks4://x:1080"))); + assert!(!should_attach_safe_resolver(Some("SOCKS5://Tor:9050"))); + } + #[test] fn safe_url_blocks_non_http_schemes() { let allow = allow_just("anywhere"); diff --git a/frontend/package.json b/frontend/package.json index 63cf872..49bf457 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.124.14", + "version": "0.124.15", "private": true, "type": "module", "scripts": {