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) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-07-11 13:46:39 +02:00
parent 91d6a50dba
commit 5b46eab73a
5 changed files with 65 additions and 13 deletions

2
backend/Cargo.lock generated
View File

@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
[[package]] [[package]]
name = "mangalord" name = "mangalord"
version = "0.124.14" version = "0.124.15"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"argon2", "argon2",

View File

@@ -1,6 +1,6 @@
[package] [package]
name = "mangalord" name = "mangalord"
version = "0.124.14" version = "0.124.15"
edition = "2021" edition = "2021"
default-run = "mangalord" default-run = "mangalord"

View File

@@ -605,16 +605,16 @@ async fn spawn_crawler_daemon(
if let Some(proxy) = &cfg.proxy { if let Some(proxy) = &cfg.proxy {
http_builder = http_builder http_builder = http_builder
.proxy(reqwest::Proxy::all(proxy).with_context(|| format!("parse proxy: {proxy}"))?); .proxy(reqwest::Proxy::all(proxy).with_context(|| format!("parse proxy: {proxy}"))?);
} else { }
// DNS-rebinding guard: reject hosts that resolve to a private/internal // DNS-rebinding guard: reject hosts that resolve to a private/internal IP,
// IP, complementing the string-level allowlist check which can't see // complementing the string-level allowlist check which can't see
// post-resolution addresses. ONLY on the direct (unproxied) path. With // post-resolution addresses. Attached on the direct path AND on http(s)
// a `socks5h://` proxy the target hostname is resolved by the proxy — // proxies (reqwest resolves the target itself there). Skipped only for SOCKS
// reqwest never resolves it — so the only name this resolver would ever // proxies, where the proxy — not reqwest — resolves the target, so the only
// see is the proxy's OWN host, which legitimately lives on a private // name this resolver would see is the proxy's OWN host (legitimately on a
// Docker IP (e.g. `tor` → 172.x). Attaching it there rejected every // private Docker IP, e.g. `tor` → 172.x); attaching it there rejected every
// fetch ("SOCKS error: failed to create underlying connection") for // fetch for zero gain. See `should_attach_safe_resolver`.
// zero security gain, since Tor can't route to internal ranges anyway. if crate::crawler::safety::should_attach_safe_resolver(cfg.proxy.as_deref()) {
http_builder = http_builder.dns_resolver(crate::crawler::safety::safe_dns_resolver()); http_builder = http_builder.dns_resolver(crate::crawler::safety::safe_dns_resolver());
} }
let http = http_builder.build().context("build crawler reqwest")?; let http = http_builder.build().context("build crawler reqwest")?;

View File

@@ -289,6 +289,40 @@ pub fn safe_dns_resolver() -> Arc<SafeResolver> {
Arc::new(SafeResolver) 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)] #[derive(Debug, thiserror::Error, PartialEq, Eq)]
pub enum UrlSafetyError { pub enum UrlSafetyError {
#[error("URL is not parseable")] #[error("URL is not parseable")]
@@ -743,6 +777,24 @@ mod tests {
assert!(err.to_string().contains("rebind.attacker")); 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] #[test]
fn safe_url_blocks_non_http_schemes() { fn safe_url_blocks_non_http_schemes() {
let allow = allow_just("anywhere"); let allow = allow_just("anywhere");

View File

@@ -1,6 +1,6 @@
{ {
"name": "mangalord-frontend", "name": "mangalord-frontend",
"version": "0.124.14", "version": "0.124.15",
"private": true, "private": true,
"type": "module", "type": "module",
"scripts": { "scripts": {