From 134ab54b3429c17d0a8bb1ae2ced394dfa6838eb Mon Sep 17 00:00:00 2001 From: fabi Date: Fri, 10 Jul 2026 22:20:25 +0200 Subject: [PATCH] fix: don't apply the SSRF DNS resolver to proxied/internal clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ff4ca96 wired SafeResolver (drops any host resolving to a private IP) into all four crawler/analysis reqwest clients. That broke every proxied fetch: with a socks5h:// proxy the target is resolved by Tor, so reqwest only ever resolves the proxy's OWN host — `tor`, on a private Docker IP (172.x) — which the resolver then rejects ("SOCKS error: failed to create underlying connection"). 100% of crawler image downloads failed. The vision clients broke the same way against `mangalord-vision` (latent; analysis was off). Fix: attach the resolver only on the crawler's direct (unproxied) path, where it genuinely guards DNS rebinding. Behind a proxy it adds no protection (Tor can't reach internal ranges and the target is invisible to reqwest) so it's omitted. Drop it from the vision clients entirely — that endpoint is an operator-configured internal service that must resolve to a private IP. Security posture is >= the pre-ff4ca96 baseline (which had no resolver at all). is_private_ip / retain_public_addrs and their tests are unchanged. Bump 0.124.13. Co-Authored-By: Claude Opus 4.8 --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- backend/src/app.rs | 28 +++++++++++++++++++--------- backend/src/bin/crawler.rs | 9 +++++++-- frontend/package.json | 2 +- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 2e924a7..24f86b0 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.124.12" +version = "0.124.13" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 4287635..74f1d50 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.124.12" +version = "0.124.13" edition = "2021" default-run = "mangalord" diff --git a/backend/src/app.rs b/backend/src/app.rs index 472bc11..6288b49 100644 --- a/backend/src/app.rs +++ b/backend/src/app.rs @@ -476,10 +476,12 @@ async fn spawn_analysis_daemon( // endpoint is a single admin-configured URL), so the policy // enforces scheme + private-IP only. .redirect(crate::crawler::safety::public_redirect_policy()) - // Filter resolved IPs: a hostname that resolves to an internal - // address (DNS rebinding) is refused at connect time, not just - // by the string check above. - .dns_resolver(crate::crawler::safety::safe_dns_resolver()) + // NOTE: deliberately no `.dns_resolver(safe_dns_resolver())`. + // The vision endpoint is a single operator-configured internal + // service (e.g. `mangalord-vision`) that legitimately resolves + // to a private Docker IP; a private-IP-rejecting resolver drops + // every call. Redirect hops stay guarded by the policy above, + // and the URL is operator-set, not attacker-controlled input. .build() .context("build analysis http client")?; let vision = crate::analysis::vision::VisionClient::new(http, cfg); @@ -506,7 +508,8 @@ async fn spawn_analysis_daemon( // uptime + vision health. .no_proxy() .redirect(crate::crawler::safety::public_redirect_policy()) - .dns_resolver(crate::crawler::safety::safe_dns_resolver()) + // No private-IP resolver: same operator-configured + // internal vision host as the analysis client above. .build() .context("build vision readiness http client")?; Some(Arc::new(crate::analysis::daemon::HttpVisionReadiness { @@ -595,10 +598,6 @@ async fn spawn_crawler_daemon( .redirect(crate::crawler::safety::safe_redirect_policy( cfg.download_allowlist.clone(), )) - // Reject any host that resolves to a private/internal IP (DNS - // rebinding), complementing the string-level allowlist/private-IP - // check which can't see post-resolution addresses. - .dns_resolver(crate::crawler::safety::safe_dns_resolver()) .cookie_provider(Arc::clone(&cookie_jar)); if let Some(ua) = &cfg.user_agent { http_builder = http_builder.user_agent(ua); @@ -606,6 +605,17 @@ 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. + 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/bin/crawler.rs b/backend/src/bin/crawler.rs index 1d4cb41..72949c5 100644 --- a/backend/src/bin/crawler.rs +++ b/backend/src/bin/crawler.rs @@ -126,8 +126,6 @@ async fn main() -> anyhow::Result<()> { .redirect(mangalord::crawler::safety::safe_redirect_policy( (*allowlist).clone(), )) - // Reject hosts that resolve to private/internal IPs (DNS rebinding). - .dns_resolver(mangalord::crawler::safety::safe_dns_resolver()) .cookie_provider(cookie_jar); if let Some(ua) = &user_agent { http_builder = http_builder.user_agent(ua); @@ -135,6 +133,13 @@ async fn main() -> anyhow::Result<()> { if let Some(proxy) = &proxy_url { http_builder = http_builder .proxy(reqwest::Proxy::all(proxy).with_context(|| format!("parse proxy URL: {proxy}"))?); + } else { + // DNS-rebinding guard, direct (unproxied) path only — see the matching + // note in `spawn_crawler_daemon` (app.rs). With a `socks5h://` proxy the + // target is resolved by the proxy, so this resolver would only ever + // reject the proxy's own private-IP host (e.g. `tor`), breaking every + // fetch for no security gain. + http_builder = http_builder.dns_resolver(mangalord::crawler::safety::safe_dns_resolver()); } let http = http_builder.build().context("build http client")?; diff --git a/frontend/package.json b/frontend/package.json index 31e792c..5de39c0 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.124.12", + "version": "0.124.13", "private": true, "type": "module", "scripts": {