From ff4ca964f5d1aef77102f25cec4512b91eec9133 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Tue, 7 Jul 2026 21:52:30 +0200 Subject: [PATCH] fix: reject private IPs after DNS resolution (SSRF/DNS-rebinding) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SSRF guard only checked the host string, so an attacker-owned domain resolving to 169.254.169.254 / 10.x (DNS rebinding, TOCTOU) bypassed it. Add a reqwest dns::Resolve (SafeResolver) that drops resolved addresses in private ranges, wired into all four crawler/analysis clients — it fires per connection so it also covers redirect hops. Also close the is_private_ip IPv6-embedding gaps (IPv4-compatible ::/96, NAT64 64:ff9b::/96, 6to4 2002::/16 all now unwrap to the embedded IPv4). Bump to 0.124.10. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- backend/src/app.rs | 9 +++ backend/src/bin/crawler.rs | 2 + backend/src/crawler/safety.rs | 148 ++++++++++++++++++++++++++++++++-- frontend/package.json | 2 +- 6 files changed, 155 insertions(+), 10 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 46540b8..7d67fa3 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.124.9" +version = "0.124.10" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index c2c436c..35b1a9f 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.124.9" +version = "0.124.10" edition = "2021" default-run = "mangalord" diff --git a/backend/src/app.rs b/backend/src/app.rs index 2dfd2eb..2cc620c 100644 --- a/backend/src/app.rs +++ b/backend/src/app.rs @@ -476,6 +476,10 @@ 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()) .build() .context("build analysis http client")?; let vision = crate::analysis::vision::VisionClient::new(http, cfg); @@ -502,6 +506,7 @@ 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()) .build() .context("build vision readiness http client")?; Some(Arc::new(crate::analysis::daemon::HttpVisionReadiness { @@ -586,6 +591,10 @@ 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); diff --git a/backend/src/bin/crawler.rs b/backend/src/bin/crawler.rs index 808dd39..b2f8f99 100644 --- a/backend/src/bin/crawler.rs +++ b/backend/src/bin/crawler.rs @@ -126,6 +126,8 @@ 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); diff --git a/backend/src/crawler/safety.rs b/backend/src/crawler/safety.rs index 8eb928d..1ada91b 100644 --- a/backend/src/crawler/safety.rs +++ b/backend/src/crawler/safety.rs @@ -31,11 +31,13 @@ //! URL string, and the byte accumulator is keyed off a generic stream. //! Easy to unit-test without a live network or browser. -use std::net::IpAddr; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; +use std::sync::Arc; use anyhow::{bail, Context}; use bytes::BytesMut; use futures_util::StreamExt; +use reqwest::dns::{Addrs, Name, Resolve, Resolving}; use reqwest::Url; /// Default per-image download cap. A page image is generally <2 MiB; @@ -179,7 +181,7 @@ fn ensure_public_target_inner(raw_url: &str) -> Result { Ok(url) } -fn is_private_ip(ip: &IpAddr) -> bool { +pub(crate) fn is_private_ip(ip: &IpAddr) -> bool { match ip { IpAddr::V4(v4) => { v4.is_loopback() @@ -193,11 +195,14 @@ fn is_private_ip(ip: &IpAddr) -> bool { || v4.octets()[0] == 0 } IpAddr::V6(v6) => { - // IPv4-mapped IPv6 (::ffff:0:0/96): unwrap to the embedded - // IPv4 and recurse so `::ffff:127.0.0.1` is caught by the - // IPv4 loopback check rather than passing through. - // `Ipv6Addr::is_loopback()` only matches `::1` exactly. - if let Some(v4) = v6.to_ipv4_mapped() { + // Any IPv6 form that *embeds* an IPv4 address (mapped + // `::ffff:0:0/96`, compatible `::/96`, NAT64 `64:ff9b::/96`, + // 6to4 `2002::/16`) is unwrapped and re-checked as its IPv4 — + // otherwise `::127.0.0.1` / `2002:7f00:1::` / `64:ff9b::7f00:1` + // would smuggle an internal IPv4 past the check (the audit's + // IPv6-embedding gap). `Ipv6Addr::is_loopback()` only matches + // `::1` exactly, so these embeddings need explicit handling. + if let Some(v4) = embedded_ipv4(v6) { return is_private_ip(&IpAddr::V4(v4)); } v6.is_loopback() @@ -210,6 +215,80 @@ fn is_private_ip(ip: &IpAddr) -> bool { } } +/// Extract the IPv4 address embedded in an IPv6 literal, for every +/// transitional encoding that can carry one: IPv4-mapped (`::ffff:0:0/96`), +/// IPv4-compatible (`::/96`, deprecated but still routable via some stacks), +/// NAT64 (`64:ff9b::/96`), and 6to4 (`2002::/16`). Returns `None` for a +/// native IPv6 address. Callers recurse into [`is_private_ip`] on the result +/// so a private IPv4 can't hide inside an IPv6 literal. +fn embedded_ipv4(v6: &Ipv6Addr) -> Option { + let seg = v6.segments(); + let low32 = |a: u16, b: u16| Ipv4Addr::new((a >> 8) as u8, (a & 0xff) as u8, (b >> 8) as u8, (b & 0xff) as u8); + // ::ffff:0:0/96 (mapped) and ::/96 (compatible) — top 96 bits zero, + // except mapped which has 0xffff at seg[5]. to_ipv4() covers both. + if seg[0..5] == [0, 0, 0, 0, 0] && (seg[5] == 0 || seg[5] == 0xffff) { + return Some(low32(seg[6], seg[7])); + } + // NAT64 64:ff9b::/96 + if seg[0] == 0x0064 && seg[1] == 0xff9b && seg[2..6] == [0, 0, 0, 0] { + return Some(low32(seg[6], seg[7])); + } + // 6to4 2002::/16 — embedded IPv4 is bits 16..48 (seg[1], seg[2]). + if seg[0] == 0x2002 { + return Some(low32(seg[1], seg[2])); + } + None +} + +/// A `reqwest::dns::Resolve` that performs normal system resolution, then +/// drops any resolved address in a private / loopback / link-local / metadata +/// range. Installed on every crawler + analysis reqwest client so a hostname +/// that resolves to an internal IP (DNS rebinding: an attacker-owned domain +/// with an `A` record for `169.254.169.254` or `10.x`) can never be connected +/// to — closing the TOCTOU gap that the string-only `ensure_public_target` +/// check leaves open. Fires per connection, so it also guards redirect hops. +#[derive(Debug, Default)] +pub struct SafeResolver; + +/// Partition resolved addresses into the public ones, rejecting when nothing +/// survives. Split out from the async `resolve` so the security-critical +/// filter is unit-testable without real DNS. +fn retain_public_addrs( + host: &str, + addrs: impl Iterator, +) -> Result, BlockedResolution> { + let public: Vec = addrs.filter(|a| !is_private_ip(&a.ip())).collect(); + if public.is_empty() { + return Err(BlockedResolution(host.to_string())); + } + Ok(public) +} + +#[derive(Debug, thiserror::Error)] +#[error("host {0} resolved only to private/blocked addresses")] +struct BlockedResolution(String); + +impl Resolve for SafeResolver { + fn resolve(&self, name: Name) -> Resolving { + Box::pin(async move { + let host = name.as_str().to_string(); + // Port 0: reqwest overrides it with the URL's port after + // resolution (same convention as reqwest's default GaiResolver). + let resolved = tokio::net::lookup_host((host.as_str(), 0)) + .await + .map_err(|e| Box::new(e) as Box)?; + let public = retain_public_addrs(&host, resolved) + .map_err(|e| Box::new(e) as Box)?; + Ok(Box::new(public.into_iter()) as Addrs) + }) + } +} + +/// Shared [`SafeResolver`] for wiring into `ClientBuilder::dns_resolver`. +pub fn safe_dns_resolver() -> Arc { + Arc::new(SafeResolver) +} + #[derive(Debug, thiserror::Error, PartialEq, Eq)] pub enum UrlSafetyError { #[error("URL is not parseable")] @@ -609,6 +688,61 @@ mod tests { assert!(matches!(err, UrlSafetyError::PrivateIp(_))); } + #[test] + fn is_private_ip_unwraps_embedded_ipv4_encodings() { + // Every IPv6 encoding that can smuggle an internal IPv4 must be + // caught. The audit flagged compatible ::/96, NAT64, and 6to4 as + // gaps past the original mapped-only handling. + for s in [ + "::ffff:127.0.0.1", // IPv4-mapped loopback + "::127.0.0.1", // IPv4-compatible loopback (was a gap) + "::ffff:10.1.2.3", // mapped RFC1918 + "::10.1.2.3", // compatible RFC1918 (was a gap) + "64:ff9b::7f00:1", // NAT64 of 127.0.0.1 (was a gap) + "64:ff9b::a01:203", // NAT64 of 10.1.2.3 + "2002:7f00:1::", // 6to4 of 127.0.0.1 (was a gap) + "2002:a01:203::", // 6to4 of 10.1.2.3 + "2002:a9fe:a9fe::", // 6to4 of 169.254.169.254 (metadata) + ] { + let ip: IpAddr = s.parse().unwrap(); + assert!(is_private_ip(&ip), "{s} must be flagged private"); + } + } + + #[test] + fn is_private_ip_allows_public_embedded_and_native_ipv6() { + // A public IPv4 embedded in IPv6, and a native public IPv6, must + // NOT be flagged — the unwrap only blocks when the embedded v4 is + // itself private. + for s in [ + "::ffff:8.8.8.8", // mapped public + "2002:808:808::", // 6to4 of 8.8.8.8 (public) + "2606:4700:4700::1111", // native public (Cloudflare) + ] { + let ip: IpAddr = s.parse().unwrap(); + assert!(!is_private_ip(&ip), "{s} must be allowed"); + } + } + + #[test] + fn retain_public_addrs_drops_private_and_errors_when_all_private() { + use std::net::{Ipv4Addr, SocketAddr}; + let pub_addr = SocketAddr::from((Ipv4Addr::new(93, 184, 216, 34), 0)); + let loopback = SocketAddr::from((Ipv4Addr::new(127, 0, 0, 1), 0)); + let metadata = SocketAddr::from((Ipv4Addr::new(169, 254, 169, 254), 0)); + + // Mixed result keeps only the public address. + let kept = + retain_public_addrs("mixed.example", [pub_addr, loopback, metadata].into_iter()) + .expect("public address survives"); + assert_eq!(kept, vec![pub_addr]); + + // All-private (DNS rebinding to internal) is rejected outright. + let err = + retain_public_addrs("rebind.attacker", [loopback, metadata].into_iter()).unwrap_err(); + assert!(err.to_string().contains("rebind.attacker")); + } + #[test] fn safe_url_blocks_non_http_schemes() { let allow = allow_just("anywhere"); diff --git a/frontend/package.json b/frontend/package.json index 1a5c882..3ff0392 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.124.9", + "version": "0.124.10", "private": true, "type": "module", "scripts": {