diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 1f493cb..8bc7ff0 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.128.23" +version = "0.128.24" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 42192e5..f509be6 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.128.23" +version = "0.128.24" edition = "2021" default-run = "mangalord" diff --git a/backend/src/bin/crawler.rs b/backend/src/bin/crawler.rs index e15cd6e..688bf30 100644 --- a/backend/src/bin/crawler.rs +++ b/backend/src/bin/crawler.rs @@ -133,13 +133,16 @@ 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()); + } + // DNS-rebinding guard: attached on the direct path AND on http(s) proxies + // (reqwest resolves the target itself there), skipped only for SOCKS proxies + // where the proxy resolves the target. Use the shared predicate so this CLI + // and the daemon (app.rs) stay in lockstep — previously the CLI attached the + // resolver only on the fully-direct path, so an http(s) proxy silently lost + // the rebinding guard. + if mangalord::crawler::safety::should_attach_safe_resolver(proxy_url.as_deref()) { + http_builder = + http_builder.dns_resolver(mangalord::crawler::safety::safe_dns_resolver()); } let http = http_builder.build().context("build http client")?; diff --git a/backend/src/crawler/intercept.rs b/backend/src/crawler/intercept.rs index 474d103..480c91a 100644 --- a/backend/src/crawler/intercept.rs +++ b/backend/src/crawler/intercept.rs @@ -82,6 +82,11 @@ pub(crate) fn verdict(url: &str) -> Verdict { }; match parsed.scheme() { "http" | "https" => {} + // Non-http(s) subresource schemes: block the ones that can reach the + // local filesystem or pivot to another protocol; continue the rest + // (data:/blob:/about: and Chrome-internal schemes) which legitimate page + // loads depend on — blocking those would wedge navigation. + "file" | "ftp" | "gopher" => return Verdict::Block, _ => return Verdict::Allow, } // Literal private IP / localhost / missing host → block (string check). @@ -210,6 +215,15 @@ mod tests { } } + #[test] + fn verdict_blocks_dangerous_non_http_schemes() { + // file:/ftp:/gopher: can reach the local filesystem or pivot protocols; + // block them explicitly rather than falling through to Allow. + for url in ["file:///etc/passwd", "ftp://internal/x", "gopher://internal:70/1"] { + assert_eq!(verdict(url), Verdict::Block, "{url} should be blocked"); + } + } + #[test] fn verdict_blocks_private_ip_literals_and_localhost() { for url in [ diff --git a/backend/src/crawler/safety.rs b/backend/src/crawler/safety.rs index 436bb30..e25b62d 100644 --- a/backend/src/crawler/safety.rs +++ b/backend/src/crawler/safety.rs @@ -507,6 +507,26 @@ mod tests { DownloadAllowlist::new().allow(host) } + #[test] + fn rejects_alternate_ipv4_encodings_of_loopback() { + // The `url` crate normalizes special-scheme IPv4 hosts per WHATWG, so + // decimal/hex/octal/short forms all collapse to 127.0.0.1 and must be + // rejected. This normalization is load-bearing (it's what stops these + // encodings smuggling past the literal-IP check) but was untested — pin + // it so a future URL-parser swap can't silently reopen the hole. + for url in [ + "http://2130706433/", // decimal 127.0.0.1 + "http://0x7f000001/", // hex + "http://0177.0.0.1/", // octal first octet + "http://127.1/", // short form + ] { + assert!( + ensure_public_target(url).is_err(), + "{url} normalizes to loopback and must be rejected" + ); + } + } + #[test] fn allow_any_admits_arbitrary_public_host() { // Operators who can't pre-enumerate a numbered-CDN fleet diff --git a/frontend/package.json b/frontend/package.json index b174fc4..feb7e47 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.128.23", + "version": "0.128.24", "private": true, "type": "module", "scripts": {