fix(proxy): add URL validation for user-supplied URLs - #25837
Conversation
…lied URLs Add validate_url() utility that resolves DNS once, validates all IPs against private network ranges, and rewrites the URL to connect to the validated IP directly. Prevents DNS rebinding by pinning to the resolved IP. Disable follow_redirects to prevent redirect-based SSRF bypasses. Applied to all user-supplied URL entry points: - Image URL fetching in chat completions - Token counter image dimension fetching - RAG file ingestion - MCP OpenAPI spec loading
Add safe_get() and async_safe_get() helpers that validate each redirect hop before following. For HTTPS, rely on TLS certificate binding instead of URL rewriting. Simplify call sites to use the new helpers.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
There was a problem hiding this comment.
Medium: HTTPS DNS rebinding window in SSRF protection
This PR adds SSRF validation for user-supplied URLs, which is a solid improvement. The implementation resolves DNS, validates all IPs, and rewrites HTTP URLs to connect to the validated IP. However, for HTTPS URLs, the original hostname-based URL is returned, relying on TLS cert validation to prevent DNS rebinding. This creates a TOCTOU window: DNS is resolved and validated in validate_url(), then resolved again by httpx when connecting. If SSL verification is disabled (via SSL_VERIFY=false or litellm.ssl_verify = False), an attacker can exploit this to reach internal services.
Additionally, _is_blocked_ip defaults to False (allow) when it fails to parse an IP address, which is the wrong safe default for a security check.
- medium: HTTPS DNS rebinding TOCTOU — litellm/proxy/common_utils/url_utils.py
- low: Fail-open on IP parse error — litellm/proxy/common_utils/url_utils.py
| # hostname — DNS rebinding can't redirect to a different server because | ||
| # the cert wouldn't match. Return the original URL. | ||
| if parsed.scheme == "https": | ||
| return url, hostname |
There was a problem hiding this comment.
Medium: DNS rebinding via TOCTOU on HTTPS URLs
For HTTPS, validate_url resolves DNS and checks the IPs, then returns the original hostname-based URL. httpx will resolve DNS again when connecting, creating a window where the DNS record can change to a private IP. The comment says TLS cert validation prevents this, but that only holds when verify=True — and litellm supports SSL_VERIFY=false (or litellm.ssl_verify = False), which disables it.
The HTTP path already rewrites the URL to connect to the validated IP. The same should be done for HTTPS, passing the original hostname via SNI/Host header so TLS still works:
# Instead of returning the original URL for HTTPS, rewrite it
# the same way as HTTP and set the Host header in safe_get.
validated_ip = addrinfo[0][4][0]
is_ipv6 = addrinfo[0][0] == socket.AF_INET6
ip_host = f"[{validated_ip}]" if is_ipv6 else validated_ipNote: connecting to an IP with HTTPS requires the caller to set SNI to the original hostname for cert validation to succeed. httpx supports this via the extensions={"sni_hostname": original_host} parameter on the request, or by configuring the transport.
| try: | ||
| ip = ip_address(addr) | ||
| except ValueError: | ||
| return False |
There was a problem hiding this comment.
Low: Fail-open default on IP parse error
If ip_address(addr) raises ValueError, this returns False (not blocked), allowing the request through. For a security gate, the safe default should be to block when the input can't be parsed.
| return False | |
| return True |
…ests Check URL scheme before calling safe_get in token counter to avoid unnecessary DNS resolution on base64-encoded image data. Add 14 unit tests for validate_url covering blocked networks, scheme validation, URL rewriting, and DNS failure handling.
There was a problem hiding this comment.
Medium: HTTPS DNS rebinding gap in SSRF protection
This PR adds solid SSRF protection for user-supplied URLs with DNS resolution validation and IP-based URL rewriting. The HTTP path is well protected against DNS rebinding by rewriting the URL to the validated IP. However, the HTTPS path returns the original hostname-based URL, creating a TOCTOU window between validate_url's DNS resolution and httpx's actual connection — an attacker-controlled domain could rebind to an internal IP between those two lookups.
- medium: HTTPS DNS rebinding TOCTOU — litellm/proxy/common_utils/url_utils.py
| # hostname — DNS rebinding can't redirect to a different server because | ||
| # the cert wouldn't match. Return the original URL. | ||
| if parsed.scheme == "https": | ||
| return url, hostname |
There was a problem hiding this comment.
Medium: DNS rebinding via HTTPS TOCTOU gap
For HTTPS, the original hostname-based URL is returned, so httpx performs a second DNS lookup at connection time. An attacker controlling a domain can return a public IP for the first lookup (passing validation) and an internal IP for the second (reaching an internal service). The comment says TLS binds the connection to the hostname, but the cert validates the hostname, not the IP — the attacker's own valid cert for their domain works regardless of the destination IP.
In practice this requires an internal HTTPS service that accepts arbitrary SNI or the client having verify=False, so exploitability is limited. Still, the HTTP path correctly avoids this by rewriting to the validated IP; the HTTPS path should do the same and pass the original hostname via SNI/Host header. For HTTPS you'd need to configure httpx to connect to the IP while still sending the original hostname for SNI — this can be done with httpx's transport-level socket_options or by setting the Host header and using an IP-based URL with verify=False scoped to the validated IP (which is tricky). A simpler approach is to use a custom DNS resolver or transport that pins the IP.
…sabled _is_blocked_ip now returns True (blocked) for unparseable addresses instead of False (allowed). HTTPS URLs are rewritten to validated IPs when ssl_verify is disabled, closing the DNS rebinding window that exists without TLS certificate binding.
Greptile SummaryThis PR adds SSRF protection across image fetching, token counting, RAG ingestion, and MCP OpenAPI spec loading by introducing a new
Confidence Score: 3/5Do not merge yet — one new P1 (Host header port regression) plus two unresolved P1s from prior threads. Three P1 issues remain open: the Host header strips the port for non-standard-port URLs (new finding), blocking DNS resolution in async paths, and SDK core importing from litellm.proxy. All three affect the primary use cases this PR is trying to protect. litellm/proxy/common_utils/url_utils.py requires the most attention: unused import, inline import, and the Host header port bug all live there.
|
| Filename | Overview |
|---|---|
| litellm/proxy/common_utils/url_utils.py | New SSRF-protection utility: validates DNS resolution, blocks private IPs, rewrites HTTP URLs to validated IPs, and manually follows redirects with per-hop validation. Three issues: Host header strips port for non-standard-port URLs (P1), unused asyncio import (P2), and inline import httpx inside a function (P2). |
| litellm/litellm_core_utils/prompt_templates/image_handling.py | Replaces direct client.get(url, follow_redirects=True) with safe_get/async_safe_get for SSRF protection; the import of litellm.proxy.common_utils.url_utils from SDK core is a pre-existing concern already flagged in this review's thread. |
| litellm/litellm_core_utils/token_counter.py | Adds URL prefix check before fetching image dimensions and routes through safe_get; logic is cleaner than the original try/except-then-base64 pattern. Same proxy-import concern as image_handling.py. |
| litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py | Swaps direct client.get(filepath) for async_safe_get in the URL branch; no logic change beyond SSRF protection. |
| litellm/rag/ingestion/base_ingestion.py | Replaces direct http_client.get(file_url) with async_safe_get for RAG file ingestion; straightforward and correct. |
| tests/mcp_tests/test_openapi_spec_path_url.py | Monkeypatches async_safe_get to bypass SSRF validation in the MCP URL test (example.local doesn't resolve in CI); the bypass is narrow and intentional. Formatting-only change to the local-file test. |
| tests/test_litellm/proxy/common_utils/test_url_utils.py | New unit tests for validate_url and _is_blocked_ip. Several tests make real DNS calls, which is already flagged in a prior review thread. |
Sequence Diagram
sequenceDiagram
participant Caller as Caller (image/RAG/MCP)
participant SG as safe_get / async_safe_get
participant VU as validate_url
participant DNS as socket.getaddrinfo
participant Server as Remote Server
Caller->>SG: get(client, url)
loop up to 10 redirect hops
SG->>VU: validate_url(url)
VU->>VU: check scheme in {http, https}
VU->>VU: parse hostname
VU->>DNS: getaddrinfo(hostname, port) [BLOCKING in async!]
DNS-->>VU: [(family, ..., (ip, port))]
VU->>VU: _is_blocked_ip(ip) for each result
alt any IP is private/blocked
VU-->>SG: raise SSRFError
SG-->>Caller: raise SSRFError
else all IPs are public
VU->>VU: HTTP rewrite URL to resolved IP
VU->>VU: HTTPS + ssl_verify return original URL
VU-->>SG: (validated_url, original_host)
end
SG->>Server: GET validated_url Host original_host follow_redirects=False
Server-->>SG: response
alt response is redirect
SG->>SG: _extract_redirect_url next url
else final response
SG-->>Caller: response
end
end
Reviews (2): Last reviewed commit: "fix: sync redirect bypass, Host header p..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def test_allows_public_https(self): | ||
| rewritten, host = validate_url("https://example.com/image.png") | ||
| assert host == "example.com" | ||
| assert rewritten == "https://example.com/image.png" | ||
|
|
||
| def test_rewrites_public_http_to_ip(self): | ||
| rewritten, host = validate_url("http://example.com/image.png") | ||
| assert host == "example.com" | ||
| assert "example.com" not in rewritten | ||
|
|
||
| def test_preserves_path_and_query(self): | ||
| rewritten, host = validate_url("http://example.com/path?key=value") | ||
| assert "/path" in rewritten | ||
| assert "key=value" in rewritten | ||
|
|
||
| def test_dns_failure_raises(self): | ||
| with pytest.raises(SSRFError, match="DNS resolution failed"): | ||
| validate_url("http://this-domain-does-not-exist-xyz123.invalid/test") |
There was a problem hiding this comment.
tests/test_litellm/
Several tests here resolve actual DNS — test_allows_public_https, test_rewrites_public_http_to_ip, test_preserves_path_and_query, and test_dns_failure_raises all call validate_url("http://example.com/…") or query a nonexistent .invalid domain, which hit the OS DNS resolver. The project rule for this folder is mock-only tests (no real network calls), so these can break in sandboxed CI environments.
Fix: monkeypatch socket.getaddrinfo to return a deterministic result for the "allows public" cases, and a socket.gaierror for the failure case.
def test_allows_public_https(self, monkeypatch):
monkeypatch.setattr(
"litellm.proxy.common_utils.url_utils.socket.getaddrinfo",
lambda *a, **kw: [(socket.AF_INET, None, None, "", ("93.184.216.34", 443))],
)
rewritten, host = validate_url("https://example.com/image.png")
assert host == "example.com"
assert rewritten == "https://example.com/image.png"Rule Used: What: prevent any tests from being added here that... (source)
| async def async_safe_get(client: Any, url: str, **kwargs: Any) -> Any: | ||
| """Async version of safe_get.""" | ||
| kwargs.pop("follow_redirects", None) | ||
| caller_headers = kwargs.pop("headers", {}) | ||
| for _ in range(_MAX_REDIRECTS): | ||
| validated_url, original_host = validate_url(url) |
There was a problem hiding this comment.
async_safe_get calls validate_url(url) on line 176, which in turn calls socket.getaddrinfo(...) (line 92) — a blocking OS syscall. Calling this directly from an async function blocks the event loop for the duration of the DNS lookup on every request. This affects every image URL (async_convert_url_to_base64) and RAG/MCP spec fetches.
Introduce a separate async_validate_url that wraps the blocking call:
import asyncio
async def async_validate_url(url: str) -> Tuple[str, str]:
parsed = urlparse(url)
# ... scheme/hostname checks ...
loop = asyncio.get_event_loop()
addrinfo = await loop.run_in_executor(
None,
lambda: socket.getaddrinfo(hostname, port or default_port, proto=socket.IPPROTO_TCP),
)
# ... rest of validation ...Then async_safe_get calls await async_validate_url(url) instead of validate_url(url).
| ) | ||
| from litellm.litellm_core_utils.default_encoding import encoding as default_encoding | ||
| from litellm.llms.custom_httpx.http_handler import _get_httpx_client | ||
| from litellm.proxy.common_utils.url_utils import safe_get |
There was a problem hiding this comment.
litellm.proxy
token_counter.py (and image_handling.py line 13) are part of the SDK core and now import from litellm.proxy.common_utils.url_utils. Users who install the bare SDK without proxy extras could hit ImportError if litellm/proxy/__init__.py or its transitive imports require unavailable deps.
Since url_utils.py only uses stdlib + import litellm, consider moving it to litellm/litellm_core_utils/url_utils.py to avoid the proxy dependency chain from SDK core code.
| # For HTTPS with SSL verification enabled, TLS certificate validation | ||
| # binds the connection to the hostname — DNS rebinding can't redirect | ||
| # to a different server because the cert wouldn't match. | ||
| # When SSL verification is disabled, this defense doesn't apply, so | ||
| # we rewrite to the validated IP like HTTP. | ||
| ssl_verify = getattr(litellm, "ssl_verify", True) | ||
| if parsed.scheme == "https" and ssl_verify is not False: | ||
| return url, hostname | ||
|
|
||
| # For HTTP, rewrite URL to connect to the validated IP directly | ||
| # to prevent DNS rebinding (no TLS to bind the connection). | ||
| validated_ip = addrinfo[0][4][0] | ||
| is_ipv6 = addrinfo[0][0] == socket.AF_INET6 | ||
| ip_host = f"[{validated_ip}]" if is_ipv6 else validated_ip | ||
|
|
||
| if port: | ||
| new_netloc = f"{ip_host}:{port}" | ||
| else: | ||
| new_netloc = ip_host | ||
|
|
||
| rewritten = urlunparse( | ||
| (parsed.scheme, new_netloc, parsed.path, parsed.params, parsed.query, "") | ||
| ) | ||
|
|
||
| return rewritten, hostname |
There was a problem hiding this comment.
ssl_verify=False)
When ssl_verify=False and scheme is https, the URL is rewritten to the resolved IP (e.g., https://93.184.216.34/image.png). httpx derives TLS SNI from the URL hostname, which is now an IP — IPs are not valid SNI values so httpx sends no SNI extension. Many CDNs and multi-tenant hosts require SNI and will return a handshake error or the wrong certificate.
A safer approach is to keep the original hostname in the HTTPS URL even with ssl_verify=False, relying on the IP blocklist guarantee at resolve time. The DNS rebinding window is unchanged since certificate validation is already disabled.
Read Location header directly instead of response.next_request (which is None when follow_redirects=False). Resolve relative redirect URLs with httpx.URL.join(). Remove unused imports.
There was a problem hiding this comment.
High: Redirect-based SSRF bypass in sync safe_get
This PR adds SSRF protection via safe_get/async_safe_get wrappers that validate each redirect hop. The protection relies on passing follow_redirects=False to the underlying httpx client. However, the sync HTTPHandler.get() (in http_handler.py) computes a _follow_redirects variable but never passes it to self.client.get(), so the client-level default of follow_redirects=True applies. This means the sync safe_get code path validates the initial URL, then httpx silently follows all redirects without any SSRF checks.
The async path works correctly because AsyncHTTPHandler.get() does forward the parameter.
| response = client.get( | ||
| validated_url, | ||
| headers={**caller_headers, "Host": original_host}, | ||
| follow_redirects=False, |
There was a problem hiding this comment.
High: Redirect-based SSRF bypass via sync HTTPHandler
safe_get passes follow_redirects=False here, but HTTPHandler.get() (the sync wrapper in http_handler.py:984) silently drops this parameter — it computes _follow_redirects but never passes it to self.client.get() at line 998. The underlying httpx.Client was created with follow_redirects=True, so redirects are followed automatically without hitting this validation loop.
An attacker can host an image at https://attacker.com/img.png that 302-redirects to http://169.254.169.254/latest/meta-data/. The initial URL passes validation, then httpx follows the redirect to the metadata endpoint without any SSRF check.
This affects convert_url_to_base64 (image_handling.py) and get_image_dimensions (token_counter.py), both of which use sync HTTPHandler clients.
The fix belongs in HTTPHandler.get() in http_handler.py:998 — pass follow_redirects=_follow_redirects to self.client.get(), matching what the async version already does at line 479.
Pass follow_redirects through in HTTPHandler.get() — previously the parameter was accepted but never forwarded to the underlying httpx client, making sync redirect protection ineffective. Include port in Host header when non-default (e.g. example.com:8080). Fix redirect loop to read Location header directly instead of response.next_request (which is None when follow_redirects=False).
| hostname = parsed.hostname | ||
| if not hostname: | ||
| raise SSRFError("URL has no hostname") | ||
|
|
||
| port = parsed.port | ||
| default_port = 443 if parsed.scheme == "https" else 80 | ||
|
|
||
| # Build the Host header value — include port when non-default | ||
| host_header = ( | ||
| hostname if (port is None or port == default_port) else f"{hostname}:{port}" | ||
| ) | ||
|
|
||
| # Resolve hostname and validate ALL addresses | ||
| try: | ||
| addrinfo = socket.getaddrinfo( | ||
| hostname, port or default_port, proto=socket.IPPROTO_TCP | ||
| ) | ||
| except socket.gaierror as e: | ||
| raise SSRFError(f"DNS resolution failed for '{hostname}': {e}") | ||
|
|
||
| if not addrinfo: | ||
| raise SSRFError(f"No addresses found for '{hostname}'") | ||
|
|
||
| for family, type_, proto, canonname, sockaddr in addrinfo: | ||
| if _is_blocked_ip(sockaddr[0]): | ||
| raise SSRFError( | ||
| f"URL targets a blocked address ({sockaddr[0]}). " | ||
| "If this is a legitimate internal service, use a direct " | ||
| "provider configuration instead of a user-supplied URL." | ||
| ) | ||
|
|
||
| # For HTTPS with SSL verification enabled, TLS certificate validation | ||
| # binds the connection to the hostname — DNS rebinding can't redirect | ||
| # to a different server because the cert wouldn't match. | ||
| # When SSL verification is disabled, this defense doesn't apply, so | ||
| # we rewrite to the validated IP like HTTP. | ||
| ssl_verify = getattr(litellm, "ssl_verify", True) | ||
| if parsed.scheme == "https" and ssl_verify is not False: | ||
| return url, host_header | ||
|
|
||
| # For HTTP, rewrite URL to connect to the validated IP directly | ||
| # to prevent DNS rebinding (no TLS to bind the connection). | ||
| validated_ip = addrinfo[0][4][0] | ||
| is_ipv6 = addrinfo[0][0] == socket.AF_INET6 | ||
| ip_host = f"[{validated_ip}]" if is_ipv6 else validated_ip | ||
|
|
||
| if port: | ||
| new_netloc = f"{ip_host}:{port}" | ||
| else: | ||
| new_netloc = ip_host | ||
|
|
There was a problem hiding this comment.
validate_url returns parsed.hostname as the second element, which is the bare hostname without any port number. safe_get / async_safe_get then use this value verbatim as the Host header. RFC 7230 §5.4 requires the port to be included when it differs from the scheme default (80 for HTTP, 443 for HTTPS), so a URL like http://api.cdn.example.com:8080/image.png would send Host: api.cdn.example.com and be rejected by servers that validate the header strictly — a real regression for any image/RAG/MCP URL on a non-standard port.
hostname = parsed.hostname
# Build the Host header value per RFC 7230: include port only when non-default
default_port = 443 if parsed.scheme == "https" else 80
if port and port != default_port:
host_header = f"{hostname}:{port}"
else:
host_header = hostnameThen return host_header instead of hostname from validate_url (and keep using the bare hostname for DNS resolution where the port is passed separately).
SDK core modules (image_handling, token_counter) should not import from litellm.proxy. Move url_utils.py to litellm_core_utils/ so bare SDK installs without proxy dependencies still work.
2 participants
Relevant issues
Adds SSRF protection for user-supplied URLs across multiple endpoints.
Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🐛 Bug Fix
Changes
1. New shared utility:
proxy/common_utils/url_utils.pyvalidate_url(url)— resolves DNS, validates all IPs against private network ranges (RFC1918, link-local, loopback, IMDS, carrier-grade NAT). For HTTP URLs, rewrites the URL to the validated IP to prevent DNS rebinding. For HTTPS, relies on TLS certificate binding.safe_get(client, url)/async_safe_get(client, url)— fetch with SSRF protection on every redirect hop. Each redirect target is validated before the request is made. Caps at 10 redirects.2. Applied to user-supplied URL entry points
image_handling.py) —convert_url_to_base64andasync_convert_url_to_base64now usesafe_get/async_safe_gettoken_counter.py) — image dimension fetching usessafe_getbase_ingestion.py) — file URL fetching usesasync_safe_getopenapi_to_mcp_generator.py) — spec URL fetching usesasync_safe_getProtection against three SSRF vectors