diff --git a/backend/Cargo.lock b/backend/Cargo.lock
index 24f86b0..39b07ef 100644
--- a/backend/Cargo.lock
+++ b/backend/Cargo.lock
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
[[package]]
name = "mangalord"
-version = "0.124.13"
+version = "0.124.14"
dependencies = [
"anyhow",
"argon2",
diff --git a/backend/Cargo.toml b/backend/Cargo.toml
index 74f1d50..1c5331e 100644
--- a/backend/Cargo.toml
+++ b/backend/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "mangalord"
-version = "0.124.13"
+version = "0.124.14"
edition = "2021"
default-run = "mangalord"
diff --git a/backend/src/crawler/intercept.rs b/backend/src/crawler/intercept.rs
index 6db78ee..474d103 100644
--- a/backend/src/crawler/intercept.rs
+++ b/backend/src/crawler/intercept.rs
@@ -9,10 +9,16 @@
//! attacker-owned hostname that resolves to `169.254.169.254`) would be
//! loaded and parsed as if it were catalog content.
//!
-//! This module installs CDP `Fetch` interception on a page so every
-//! **Document** (main-frame) request and redirect is re-validated through the
-//! same [`ensure_public_target`] + resolved-IP check the reqwest paths use,
-//! failing any that target an internal address.
+//! This module installs CDP `Fetch` interception on a page so **every** request
+//! it issues — the main-frame Document, its redirects, *and every subresource*
+//! (`
`, `fetch()`/XHR, media, …) — is re-validated through the same
+//! [`ensure_public_target`] + resolved-IP check the reqwest paths use, failing
+//! any that target an internal address. Intercepting only the Document would
+//! leave a scraped page free to pull `
` or
+//! `fetch('http://postgres:5432')` straight past the guard, so no resource type
+//! is exempt. (WebSocket handshakes are not surfaced by CDP `Fetch`, so `ws://`
+//! internal targets remain out of this hook's reach — the reqwest-layer
+//! resolver does not see them either; documented as a known gap.)
//!
//! **Opt-in / default-off.** Enabling `Fetch` means every intercepted request
//! *must* be resolved by a live handler or the navigation hangs, so this is a
@@ -20,9 +26,9 @@
//! `CRAWLER_SSRF_INTERCEPT` (default `false`) and the wiring has **not** been
//! exercised against a real Chromium in CI — validate with a manual crawl
//! before enabling in production. When disabled, [`open_page`] is byte-for-byte
-//! the previous `browser.new_page(url)` behavior. Even when enabled, a failure
-//! to install the guard falls back to an unguarded navigation rather than
-//! failing the crawl.
+//! the previous `browser.new_page(url)` behavior. When enabled, the guard is
+//! **fail-closed**: if interception can't be installed, [`open_page`] closes the
+//! blank page and returns the error rather than navigating unguarded.
use std::net::IpAddr;
use std::sync::atomic::{AtomicBool, Ordering};
@@ -32,7 +38,7 @@ use chromiumoxide::cdp::browser_protocol::fetch::{
ContinueRequestParams, EnableParams, EventRequestPaused, FailRequestParams, RequestPattern,
RequestStage,
};
-use chromiumoxide::cdp::browser_protocol::network::{ErrorReason, ResourceType};
+use chromiumoxide::cdp::browser_protocol::network::ErrorReason;
use chromiumoxide::error::Result as CdpResult;
use chromiumoxide::Page;
use futures_util::StreamExt;
@@ -129,26 +135,35 @@ pub async fn open_page(browser: &Browser, url: &str) -> CdpResult {
}
let page = browser.new_page("about:blank").await?;
if let Err(e) = install_navigation_guard(&page).await {
- // Fail open on install error: navigate unguarded rather than wedge the
- // crawl. The reqwest-layer resolver still covers image downloads.
- tracing::warn!(url = %url, error = %e, "SSRF navigation guard failed to install; navigating unguarded");
+ // Fail closed: an unguarded page could be redirected (or pull a
+ // subresource) to an internal target, so refuse to navigate. Close the
+ // blank page and surface the error so the caller aborts this fetch.
+ tracing::warn!(url = %url, error = %e, "SSRF navigation guard failed to install; aborting navigation (fail-closed)");
+ let _ = page.close().await;
+ return Err(e);
}
page.goto(url).await?;
Ok(page)
}
-/// Enable `Fetch` for Document requests on `page` and spawn a task that
+/// The `Fetch` interception patterns to register. A single pattern with **no**
+/// `resource_type` constraint matches every request the page makes — Document,
+/// Image, Fetch/XHR, media, everything — so subresources to internal targets are
+/// re-validated too, not only the main-frame navigation. Restricting this to
+/// `ResourceType::Document` (the previous behavior) left every `
`/`fetch()`
+/// subresource unguarded, which is the SSRF hole this closes. Pinned to the
+/// request stage so each request is paused once, before it leaves the browser.
+fn interception_patterns() -> Vec {
+ vec![RequestPattern::builder()
+ .request_stage(RequestStage::Request)
+ .build()]
+}
+
+/// Enable `Fetch` for all requests on `page` and spawn a task that
/// continues/fails each paused request per [`is_blocked`].
async fn install_navigation_guard(page: &Page) -> CdpResult<()> {
- // Intercept only main-frame Document requests at the request stage — a
- // navigation plus its redirects, nothing else. Keeps the paused-request
- // volume tiny so the handler can't become a page-load bottleneck.
- let pattern = RequestPattern::builder()
- .resource_type(ResourceType::Document)
- .request_stage(RequestStage::Request)
- .build();
page.execute(EnableParams {
- patterns: Some(vec![pattern]),
+ patterns: Some(interception_patterns()),
handle_auth_requests: None,
})
.await?;
@@ -244,6 +259,21 @@ mod tests {
assert!(!is_blocked("about:blank").await);
}
+ #[test]
+ fn interception_covers_all_resource_types() {
+ // Regression guard for the SSRF subresource hole: the CDP Fetch pattern
+ // must NOT be constrained to Document, or `
`/`fetch()`/XHR
+ // subresources to internal targets slip past `is_blocked`. A pattern
+ // with no `resource_type` set intercepts every request type.
+ let patterns = interception_patterns();
+ assert_eq!(patterns.len(), 1);
+ assert!(
+ patterns[0].resource_type.is_none(),
+ "interception must cover all resource types (subresources included), not just Document"
+ );
+ assert_eq!(patterns[0].request_stage, Some(RequestStage::Request));
+ }
+
#[test]
fn enabled_toggle_roundtrips() {
// Global; restore afterwards so other tests see the default.
diff --git a/frontend/package.json b/frontend/package.json
index 5de39c0..63cf872 100644
--- a/frontend/package.json
+++ b/frontend/package.json
@@ -1,6 +1,6 @@
{
"name": "mangalord-frontend",
- "version": "0.124.13",
+ "version": "0.124.14",
"private": true,
"type": "module",
"scripts": {