feat(crawler): re-validate redirect hops to close SSRF via 3xx
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -226,6 +226,73 @@ pub enum UrlSafetyError {
|
||||
HostNotAllowed(String),
|
||||
}
|
||||
|
||||
/// Maximum number of redirects the crawler will follow before giving up.
|
||||
/// Matches reqwest's historical default; every hop is re-validated by
|
||||
/// [`check_redirect_hop`], so the cap is a belt-and-braces stop against a
|
||||
/// redirect loop rather than the primary SSRF defence.
|
||||
pub const MAX_REDIRECTS: usize = 10;
|
||||
|
||||
/// Why a redirect hop was refused.
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum RedirectError {
|
||||
#[error("redirect chain exceeded {0} hops")]
|
||||
TooManyHops(usize),
|
||||
#[error("redirect target rejected: {0}")]
|
||||
Unsafe(#[from] UrlSafetyError),
|
||||
}
|
||||
|
||||
/// Decide whether a single redirect hop is safe to follow.
|
||||
///
|
||||
/// `is_safe_url` only inspects the *initial* URL a caller hands to reqwest;
|
||||
/// without re-validation an allowlisted CDN that answers `302 ->
|
||||
/// http://169.254.169.254/...` or `-> http://127.0.0.1:5432/` would be
|
||||
/// followed transparently (reqwest's default policy follows up to 10
|
||||
/// redirects). This re-runs the full allowlist + private-IP + scheme check on
|
||||
/// each hop and enforces [`MAX_REDIRECTS`]. `completed_hops` is the number of
|
||||
/// URLs already visited in the chain (reqwest's `attempt.previous().len()`).
|
||||
pub fn check_redirect_hop(
|
||||
next_url: &str,
|
||||
completed_hops: usize,
|
||||
allow: &DownloadAllowlist,
|
||||
) -> Result<(), RedirectError> {
|
||||
if completed_hops >= MAX_REDIRECTS {
|
||||
return Err(RedirectError::TooManyHops(completed_hops));
|
||||
}
|
||||
is_safe_url(next_url, allow)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Build a reqwest redirect policy that re-validates every hop against the
|
||||
/// download allowlist (see [`check_redirect_hop`]). Use for the crawler image
|
||||
/// clients, which fetch attacker-influenced URLs.
|
||||
pub fn safe_redirect_policy(allow: DownloadAllowlist) -> reqwest::redirect::Policy {
|
||||
reqwest::redirect::Policy::custom(move |attempt| {
|
||||
let hops = attempt.previous().len();
|
||||
match check_redirect_hop(attempt.url().as_str(), hops, &allow) {
|
||||
Ok(()) => attempt.follow(),
|
||||
Err(e) => attempt.error(e),
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// Build a reqwest redirect policy that re-validates every hop with
|
||||
/// [`ensure_public_target`] (scheme + private-IP, no allowlist). Use for
|
||||
/// single-endpoint clients (the analysis vision endpoint / its probe) where
|
||||
/// there is no per-deployment allowlist but a redirect into the deployment's
|
||||
/// internal network must still be refused.
|
||||
pub fn public_redirect_policy() -> reqwest::redirect::Policy {
|
||||
reqwest::redirect::Policy::custom(move |attempt| {
|
||||
let hops = attempt.previous().len();
|
||||
if hops >= MAX_REDIRECTS {
|
||||
return attempt.error(RedirectError::TooManyHops(hops));
|
||||
}
|
||||
match ensure_public_target(attempt.url().as_str()) {
|
||||
Ok(()) => attempt.follow(),
|
||||
Err(e) => attempt.error(RedirectError::Unsafe(e)),
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// Drain a byte stream into a single buffer, bailing out as soon as
|
||||
/// the running total exceeds `max_bytes`. Generic over the stream so
|
||||
/// it's testable without a live HTTP response.
|
||||
@@ -578,6 +645,54 @@ mod tests {
|
||||
assert!(is_safe_url("https://CDN.EXAMPLE.com/x.jpg", &allow).is_ok());
|
||||
}
|
||||
|
||||
// --- redirect-hop re-validation (SSRF via 3xx) ---
|
||||
|
||||
#[test]
|
||||
fn redirect_hop_allows_listed_public_target() {
|
||||
let allow = allow_just("cdn.example.com");
|
||||
assert!(check_redirect_hop("https://cdn.example.com/next.jpg", 1, &allow).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redirect_hop_blocks_private_ip_target() {
|
||||
// The core SSRF case: an allowlisted CDN 302s to the cloud metadata
|
||||
// service / an intra-compose port. Must be refused mid-chain.
|
||||
let allow = allow_just("cdn.example.com");
|
||||
for url in ["http://169.254.169.254/", "http://127.0.0.1:5432/", "http://10.0.0.1/"] {
|
||||
let err = check_redirect_hop(url, 1, &allow).unwrap_err();
|
||||
assert!(
|
||||
matches!(err, RedirectError::Unsafe(UrlSafetyError::PrivateIp(_))),
|
||||
"expected PrivateIp for {url}, got {err:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redirect_hop_blocks_off_allowlist_public_host() {
|
||||
// Per the strict policy: a redirect to an unlisted *public* host is
|
||||
// also refused (allow_any covers the numbered-CDN case instead).
|
||||
let allow = allow_just("cdn.example.com");
|
||||
let err = check_redirect_hop("https://evil.example.org/x", 1, &allow).unwrap_err();
|
||||
assert!(matches!(err, RedirectError::Unsafe(UrlSafetyError::HostNotAllowed(_))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redirect_hop_blocks_bad_scheme_target() {
|
||||
let allow = DownloadAllowlist::allow_any();
|
||||
let err = check_redirect_hop("file:///etc/passwd", 1, &allow).unwrap_err();
|
||||
assert!(matches!(err, RedirectError::Unsafe(UrlSafetyError::BadScheme(_))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redirect_hop_caps_chain_length() {
|
||||
let allow = allow_just("cdn.example.com");
|
||||
// A safe target is still refused once the hop cap is reached, so a
|
||||
// redirect loop can't spin forever.
|
||||
let err = check_redirect_hop("https://cdn.example.com/x", MAX_REDIRECTS, &allow)
|
||||
.unwrap_err();
|
||||
assert!(matches!(err, RedirectError::TooManyHops(n) if n == MAX_REDIRECTS));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn accumulate_capped_returns_full_body_under_cap() {
|
||||
let chunks: Vec<Result<bytes::Bytes, std::io::Error>> = vec![
|
||||
|
||||
Reference in New Issue
Block a user