fix(crawler): re-validate redirect hops and strip credentials cross-origin - #2371
fix(crawler): re-validate redirect hops and strip credentials cross-origin#2371namann5 wants to merge 6 commits into
Conversation
…rigin Fixes utksh1#2369 crawl_target() followed redirects via httpx's automatic handling with no re-validation of any hop, and forwarded operator credentials (including the decrypted vault Authorization header injected by the executor) and cookies to every redirect destination. Two defenses are added in backend/secuscan/crawler.py: - Redirect hops are re-validated against the network policy engine before being fetched, so a hostile or compromised seed cannot pivot the crawler into cloud-metadata, loopback, private/CGNAT, or IPv6 link-local/ULA ranges (SSRF). - Credentials are only sent to the seed origin; they are stripped on any cross-origin redirect, so vault credentials cannot be exfiltrated to an attacker-controlled host (browser-equivalent behavior). Redirects are now followed manually (follow_redirects=False) to keep the existing max_redirects and max_size constraints intact.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f7aa95067
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
utksh1
left a comment
There was a problem hiding this comment.
Request changes: redirect validation checks the hostname through check_access, but the follow-up HTTP request resolves the hostname again. That leaves a DNS-rebinding window between policy validation and the actual fetch. Resolve and pin the address for every redirect hop, or use a transport that connects to the validated address while preserving the Host header; add a rebinding regression test.
|
@utksh1 Please review |
_validate_redirect_target now resolves DNS up front and validates every returned IP against the network policy, returning the validated address. crawl_target rewrites redirect-hop URLs to use the pinned IP with the original hostname in the Host header, closing the DNS-rebinding window between policy validation and the actual HTTP fetch. Add tests for DNS resolution, validated IP return, resolution failure, and IP-pinning behavior on redirect hops.
…t-ssrf-credential-exfil
1de1f1b to
5da878a
Compare
|
@utksh1 All admin review comments have been addressed and the branch is rebased onto the latest main (including the undici override fix), so all required checks are green and the PR is mergeable. Requesting re-review. Summary of changes in this PR:
|
Description
crawl_target()inbackend/secuscan/crawler.pyfollowed HTTP redirects automatically (httpx.AsyncClient(follow_redirects=True, max_redirects=10, ...)) with no re-validation of any hop, and it forwarded operator-supplied credentials — including the decrypted vaultAuthorization: Basic ...header injected by the executor (executor.py:_hydrate_inputs_with_execution_context) — plus session cookies to every redirect destination.This PR hardens redirect handling in the crawler:
SSRF via redirect is closed. Redirect hops are re-validated against the network policy engine before they are fetched, so a hostile or compromised seed cannot pivot the crawler into cloud-metadata (
169.254.169.254), loopback, private/CGNAT, or IPv6 link-local/ULA ranges that were never authorized. This matches how the seed target is already validated by_enforce_guardrails, and is active whenenforce_network_policyis enabled (the default). Validation is fail-closed: if the policy engine errors, the redirect is blocked.Credential exfiltration via cross-origin redirect is stopped. Credentials (
extra_headerssuch asAuthorization, plus cookies) are only sent to the seed origin. On any cross-origin redirect (different scheme, host, or effective port), they are stripped — mirroring browser behavior. This prevents an attacker who controls a redirect from an authorized target from harvesting the operator's decrypted vault credentials at their own listener.Redirects are now followed manually (
follow_redirects=False) to preserve the existingmax_redirectsandmax_sizeconstraints while adding per-hop checks.Related Issues
Fixes #2369
Type of Change
How Has This Been Tested?
testing/backend/unit/test_crawler_redirect_security.py(16 tests) covering:_is_same_originsemantics (scheme / host / port, case-insensitivity, malformed-port fail-closed behavior)._validate_redirect_targetblocking cloud-metadata, unsupported schemes, missing hostnames, and failing closed on engine errors.crawl_targetaborting a redirect into169.254.169.254before the destination is fetched (asserts the metadata endpoint is never requested).crawl_targetstrippingAuthorization/cookies on a cross-origin redirect while preserving them on a same-origin redirect.max_redirects(raiseshttpx.TooManyRedirects).ruff check backend testing/backend— passes.test_crawler_limits.py,test_crawler_helpers.py,test_crawler_surface_parsing.py,test_tls_verification.py,test_crawler_plugin.py) — all pass except one pre-existing, unrelated metadata assertion (test_crawler_target_field_requires_http_url) that also fails onmainand is not executed by CI.testing/backend/unitsuite: 2311 passed; the 26 local failures are pre-existing environment-specific ones (parser sandbox subprocess / ICMP / scheduler timeout) that fail identically onmainin this Windows/Python 3.10 environment and are unrelated to this change.Checklist