fix: tighten SSRF interceptor residuals
- verdict() now blocks file:/ftp:/gopher: subresources instead of allowing all non-http(s) schemes (keeps data:/blob:/internal for page loads). - bin/crawler.rs uses should_attach_safe_resolver() so an http(s) proxy keeps the DNS-rebinding guard, matching the daemon. - Regression test pins the url crate's decimal/hex/octal/short IPv4 normalization to loopback (ensure_public_target rejects them). Tested. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.128.23"
|
||||
version = "0.128.24"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.128.23"
|
||||
version = "0.128.24"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -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")?;
|
||||
|
||||
|
||||
@@ -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 [
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mangalord-frontend",
|
||||
"version": "0.128.23",
|
||||
"version": "0.128.24",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
Reference in New Issue
Block a user