fix(aiohttp): block private/metadata IPs in api_base to close SSRF gap from PR #26264 (CWE-918) - #28246
Conversation
… from BerriAI#26264 (CWE-918) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR closes an SSRF gap in
Confidence Score: 5/5The change is well-scoped to the aiohttp handler's SSRF guard and does not touch any other request path or authentication logic; safe to merge. The SSRF mitigation logic is correct and thorough — all DNS answers are validated, IPv4-mapped IPv6 is unwrapped, No files require special attention; both changed files are isolated to the SSRF guard implementation and its tests.
|
| Filename | Overview |
|---|---|
| litellm/llms/custom_httpx/aiohttp_handler.py | Adds _BLOCKED_NETWORKS, _is_blocked_address, _assert_not_private_url, and _SSRFGuardResolver to close the SSRF gap; the resolver uses deprecated asyncio.get_event_loop() inside an async method (should be get_running_loop()). |
| tests/test_litellm/llms/test_aiohttp_ssrf_protection.py | New test file with 9 mock-only tests covering the SSRF guard and custom resolver; uses deprecated asyncio.get_event_loop().run_until_complete() pattern that emits DeprecationWarnings in Python 3.10+ and will break in future Python versions. |
Reviews (5): Last reviewed commit: "fix(aiohttp): add SSRFGuardResolver to c..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
PR overviewHigh: Redirects can bypass the SSRF guardThis PR adds private-IP checks before aiohttp/httpx calls and a guarded aiohttp resolver. Redirects are still followed by default, so an attacker-controlled public Security review
Risk: 8/10 |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
|
🤖 litellm-agent: This PR is currently BLOCKED from merge. Score: 2/5 ❌ Why blocked:
Details: Score docked for: 1 PR-related CI failure (Greptile gate: score 3/5 below required 4/5 — request a Greptile review ( Fix the issues above and push an update — the bot will re-review automatically.
|
…t block, IPv4-mapped IPv6, fe80+0.0.0.0 - Check all getaddrinfo answers (not just [0]) to prevent A-record rotation bypass - Add _assert_not_private_url to sync path (_make_common_sync_call) - Pass allow_redirects=False to session.post to prevent redirect-based bypass - Unwrap IPv4-mapped IPv6 addresses (::ffff:10.x.x.x) before network check - Add 0.0.0.0/8 and fe80::/10 (IPv6 link-local) to blocked networks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| if not hostname: | ||
| return | ||
| try: | ||
| answers = socket.getaddrinfo(hostname, None) |
There was a problem hiding this comment.
High: DNS rebinding can bypass the private-IP check
This preflight resolves the hostname once, but the actual aiohttp/httpx request resolves it again later. A caller who controls api_base can use a domain that returns a public address for this check and then a private or metadata address for the client connection; enforce the block in the client resolver/transport at connect time, or pin the request to the vetted resolved address.
|
🤖 litellm-agent: This PR is currently BLOCKED from merge. Score: 2/5 ❌ Why blocked:
Details: Score docked for: 1 PR-related CI failure (Greptile gate: score 3/5 below required 4/5 — request a Greptile review ( Fix the issues above and push an update — the bot will re-review automatically.
|
… restore redirects Replace allow_redirects=False with _SSRFGuardResolver (aiohttp.abc.AbstractResolver) that validates every resolved IP at TCP-connection time — covering redirect targets too. This eliminates the DNS-rebinding TOCTOU window inherent in a separate preflight check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
||
| for i in range(max(max_retry_on_unprocessable_entity_error, 1)): | ||
| try: | ||
| response = await async_client_session.post( |
There was a problem hiding this comment.
High: Redirects can bypass the SSRF guard
ClientSession.post() follows redirects by default, but _assert_not_private_url(api_base) only checks the original URL. An attacker who controls api_base can point it at a public host that returns a redirect to http://169.254.169.254/... or another private IP; validate each redirect target before following it, or disable redirects here and in the sync HTTPHandler path that uses follow_redirects=True.
1 participant
Relevant issues
Closes SSRF gap in
aiohttp_handler.py(CWE-918, findings #13–14).Summary
PR #26264 blocked private IPs in the main HTTP handler but
aiohttp_handler.pywas notcovered. User-controlled
api_baseis passed directly tosession.post(url=api_base, ...)atline ~196 without IP validation, allowing requests to
169.254.169.254(AWS IMDS),10.x.x.xinternal hosts, loopback, or CGNAT addresses.Fix
Added
_assert_not_private_url()which resolves the hostname viasocket.getaddrinfo()andchecks against RFC-1918 / link-local / loopback / CGNAT networks before the request is made.
Called once per invocation of
_make_common_async_call(), before the retry loop.Networks blocked
10.0.0.0/8,172.16.0.0/12,192.168.0.0/16(RFC-1918)169.254.0.0/16(link-local / AWS IMDSv1)127.0.0.0/8(loopback)100.64.0.0/10(CGNAT)::1/128,fc00::/7(IPv6 loopback / ULA)DNS failures are not blocked — the request fails naturally with a connection error.
tests/test_litellm/llms/test_aiohttp_ssrf_protection.pyuv run pytest tests/test_litellm/llms/test_aiohttp_ssrf_protection.py -v)uv run black .— no changes neededuv run ruff check— all checks passedaiohttp_handler.pySSRF guard + testType
🐛 Bug Fix
What changed: Added _assert_not_private_url() to aiohttp_handler.py + 9 tests (all passing).
The key improvement over the playbook: the ValueError-inside-try bug was fixed so private-IP
errors and DNS failures are handled separately.