Conversation
_is_http_git_repo runs from the Strix host itself (not the sandbox) to check whether a target URL looks like a git repo. It followed redirects and treated HTTP 401 as a positive "is a git repo" signal, so an attacker-influenced target string could make Strix's own infrastructure probe internal/private services (a classic SSRF pattern), including via a 401-gated redirect chain. Add _is_ssrf_safe_host to reject targets that resolve to loopback/private/link-local/reserved/multicast addresses before probing, disable redirect-following on the probe request, and only treat a genuine 200 response with the expected git content-type as a positive signal. Fixes usestrix#1132 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
| with requests.get(check_url, headers={"User-Agent": "git/2.43.0"}, timeout=10) as resp: | ||
| if resp.status_code >= 400: | ||
| return resp.status_code == 401 | ||
| with requests.get( |
There was a problem hiding this comment.
The safety check resolves the hostname but keeps only a boolean result. requests.get then connects using the original hostname, causing a separate DNS lookup. An attacker-controlled hostname can return a public address during validation and rebind to a private or link-local address for the connection, allowing this host-side probe to reach internal infrastructure. The request must connect to a validated address while preserving the original hostname for HTTP and TLS, or enforce the address policy when connecting.
How this was verified: Attacker-controlled scan targets reach this function, and the validated addresses are neither returned nor pinned before the subsequent request resolves the hostname again.
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/interface/utils.py
Line: 1169
Comment:
**DNS rebinding bypasses guard**
The safety check resolves the hostname but keeps only a boolean result. `requests.get` then connects using the original hostname, causing a separate DNS lookup. An attacker-controlled hostname can return a public address during validation and rebind to a private or link-local address for the connection, allowing this host-side probe to reach internal infrastructure. The request must connect to a validated address while preserving the original hostname for HTTP and TLS, or enforce the address policy when connecting.
**How this was verified:** Attacker-controlled scan targets reach this function, and the validated addresses are neither returned nor pinned before the subsequent request resolves the hostname again.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.…g bypass The previous SSRF guard validated the target hostname and then let requests.get() resolve and connect to it separately. That leaves a DNS-rebinding window: a malicious DNS server can answer the validation lookup with a public address and the connection's own lookup, moments later, with a private one, since nothing pins the two lookups together. Resolve the hostname once via _resolve_pinned_probe_ip, validate that address, and connect directly to it with urllib3's connection pools (passing server_hostname/assert_hostname for TLS SNI and certificate hostname verification, and an explicit Host header) instead of handing the hostname back to an HTTP client that would re-resolve it. Adds a regression test that simulates a rebinding DNS server (public address on the first lookup, private on any later one) and asserts the probe still connects to the address it already validated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for the review — good catch. The original guard validated the hostname and then let Pushed a follow-up commit that closes this: the hostname is now resolved exactly once via Added Full check-all equivalent (ruff format/check, mypy, pyright, bandit, pytest) re-run clean on the updated diff. |
Summary
_is_http_git_repoprobes a scan target URL from the Strix host itself (not the sandbox) to infer whether it's a git repo. It followed HTTP redirects by default and treated any401response as a positive "yes, this is a git repo" signal._is_ssrf_safe_host, which rejects hosts that resolve (directly or via DNS) to loopback/private/link-local/reserved/multicast addresses, applied before the probe request is made.allow_redirects=False).200response with the expectedx-git-upload-pack-advertisementcontent-type now counts as confirmation;401/403/other4xxno longer do.Fixes
Fixes #1132
Test plan
tests/test_ssrf_guard.pycovering: rejection of loopback/private/link-local/reserved/multicast IP literals, rejection of hostnames that resolve to those ranges (mocked DNS), rejection of unresolvable hostnames, that_is_http_git_reponever issues a request for a disallowed host, that401no longer counts as a positive signal, that redirects are not followed, and that a genuine200git response is still accepted.uv run ruff format --check/uv run ruff checkon changed files — cleanuv run mypy strix/interface/utils.py— cleanuv run pyright strix/interface/utils.py— no new findings (diffed againstmain; all 82 pre-existing findings unrelated to this change)uv run bandit -c pyproject.toml strix/interface/utils.py— no issuesuv run pytest tests/test_ssrf_guard.py tests/test_local_sources.py tests/test_api_spec_targets.py— 58 passedpytest tests/ -k "target or ssrf or interface or local_source or api_spec") — 135 passed