fix: guard all browser subresources against SSRF and fail closed
The CDP Fetch interceptor only paused main-frame Document requests, so a scraped page's <img>/fetch()/XHR subresources to internal targets (169.254.169.254, postgres:5432, RFC1918) reached those hosts unguarded. Drop the ResourceType::Document constraint so every request runs through is_blocked. Also make open_page fail-closed: if the guard can't install, close the blank page and return the error instead of navigating unguarded. Co-Authored-By: Claude Opus 4.8 (1M context) <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.124.13"
|
||||
version = "0.124.14"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.124.13"
|
||||
version = "0.124.14"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -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*
|
||||
//! (`<img>`, `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 `<img src="http://169.254.169.254/…">` 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<Page> {
|
||||
}
|
||||
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 `<img>`/`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<RequestPattern> {
|
||||
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 `<img>`/`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.
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mangalord-frontend",
|
||||
"version": "0.124.13",
|
||||
"version": "0.124.14",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
Reference in New Issue
Block a user