From 5b46eab73a380b9a277ee0c1e60365841453c0b1 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 11 Jul 2026 13:46:39 +0200 Subject: [PATCH] fix: keep SSRF DNS resolver on http(s)-proxy crawler paths Commit 134ab54 dropped the safe DNS resolver for any configured proxy, but that guard is only pointless for SOCKS proxies (where the proxy resolves the target). On an http(s):// proxy reqwest still resolves the target locally, so a hostname resolving to a private IP would be reachable. Narrow the exemption to SOCKS-only via should_attach_safe_resolver. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- backend/src/app.rs | 20 +++++++------- backend/src/crawler/safety.rs | 52 +++++++++++++++++++++++++++++++++++ frontend/package.json | 2 +- 5 files changed, 65 insertions(+), 13 deletions(-) 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": {