mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-06 04:23:40 +00:00
fix(security): close P1 recursion-DoS + P2 hostname leak in SSRF fixes
Greptile follow-ups on the prior commit: - (P1) ``is_request_body_safe`` recursed into ``litellm_embedding_config`` with no depth bound, so a request body 1000 levels deep could exhaust Python's call stack and surface a 500 ``RecursionError``. Refactored the check to be iterative (single-level descent into a fixed list of nested-config keys) and extracted the per-dict banned-param scan into a helper that's shared between the root and the nested call sites. Also fixes the ``recursive_detector`` CI job that was triggered by the recursive-by-name pattern. - (P2) ``assert_same_origin`` error messages identified the mismatching component but echoed the ``expected`` host and the candidate hostname back to the caller. In the SSRF threat model the caller is the attacker, so reflecting that information was a secondary leak of operator infrastructure. Messages now identify only *which* component mismatched (scheme / host / port) without naming names. - (P2) ``_NESTED_CONFIG_KEYS`` was defined after the function that used it. Hoisted the constant (and the new ``_BANNED_REQUEST_BODY_PARAMS`` tuple) above the function for readability. Adds a 1000-level-deep nested config test that asserts no ``RecursionError`` and a hostname-leak test that asserts no operator host appears in the rejection message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -212,34 +212,32 @@ def assert_same_origin(candidate_url: str, expected_url: str) -> None:
|
||||
Hostnames are compared case-insensitively. Default ports are made
|
||||
explicit (HTTP→80, HTTPS→443) so ``https://api.example.com:443/...``
|
||||
and ``https://api.example.com/...`` are treated as the same origin.
|
||||
|
||||
Error messages identify *which* component mismatched but never echo
|
||||
the operator's ``expected`` host or the candidate's hostname back to
|
||||
the caller — in the SSRF threat model the caller is the attacker,
|
||||
and reflecting host info would be a secondary leak of operator
|
||||
infrastructure details.
|
||||
"""
|
||||
candidate = urlparse(candidate_url)
|
||||
expected = urlparse(expected_url)
|
||||
|
||||
if candidate.scheme not in _ALLOWED_SCHEMES:
|
||||
raise SSRFError(f"URL scheme '{candidate.scheme}' is not allowed")
|
||||
raise SSRFError("URL scheme is not allowed")
|
||||
|
||||
if candidate.scheme != expected.scheme:
|
||||
raise SSRFError(
|
||||
"Origin mismatch: scheme "
|
||||
f"{candidate.scheme!r} != expected {expected.scheme!r}"
|
||||
)
|
||||
raise SSRFError("Origin mismatch on scheme")
|
||||
|
||||
candidate_host = _normalize_host(candidate.hostname or "")
|
||||
expected_host = _normalize_host(expected.hostname or "")
|
||||
if not candidate_host or candidate_host != expected_host:
|
||||
raise SSRFError(
|
||||
"Origin mismatch: host "
|
||||
f"{candidate.hostname!r} != expected {expected.hostname!r}"
|
||||
)
|
||||
raise SSRFError("Origin mismatch on host")
|
||||
|
||||
default_port = 443 if candidate.scheme == "https" else 80
|
||||
candidate_port = candidate.port if candidate.port is not None else default_port
|
||||
expected_port = expected.port if expected.port is not None else default_port
|
||||
if candidate_port != expected_port:
|
||||
raise SSRFError(
|
||||
"Origin mismatch: port " f"{candidate_port} != expected {expected_port}"
|
||||
)
|
||||
raise SSRFError("Origin mismatch on port")
|
||||
|
||||
|
||||
_MAX_REDIRECTS = 10
|
||||
|
||||
@@ -167,6 +167,81 @@ def _allow_model_level_clientside_configurable_parameters(
|
||||
)
|
||||
|
||||
|
||||
# Config dicts whose entries are spread as ``**dict`` into outbound LLM
|
||||
# API calls. ``litellm_embedding_config`` is consumed by the Milvus
|
||||
# vector store transformer; future nested-config keys with the same
|
||||
# threat shape should be added here.
|
||||
_NESTED_CONFIG_KEYS: Tuple[str, ...] = ("litellm_embedding_config",)
|
||||
|
||||
# Banned root-level params. Same list applies to every entry in
|
||||
# ``_NESTED_CONFIG_KEYS`` because those dicts get spread as ``**kwargs``
|
||||
# into the same outbound calls.
|
||||
_BANNED_REQUEST_BODY_PARAMS: Tuple[str, ...] = (
|
||||
"api_base",
|
||||
"base_url",
|
||||
"user_config",
|
||||
"aws_sts_endpoint",
|
||||
"aws_web_identity_token",
|
||||
"aws_role_name",
|
||||
"vertex_credentials",
|
||||
# Endpoint-targeting fields that retarget the outbound request or
|
||||
# an observability callback. An attacker-controlled value either
|
||||
# exfiltrates the request payload (incl. messages + admin-set
|
||||
# tokens) to the attacker's host, or coerces the proxy into
|
||||
# authenticating against the attacker's host with admin secrets.
|
||||
"aws_bedrock_runtime_endpoint",
|
||||
"langsmith_base_url",
|
||||
"langfuse_host",
|
||||
"posthog_host",
|
||||
"braintrust_host",
|
||||
"slack_webhook_url",
|
||||
# Provider-specific endpoint overrides that flow into the outbound
|
||||
# request via ``optional_params``. Same threat as ``api_base``:
|
||||
# ``s3_endpoint_url`` redirects Bedrock file uploads to attacker
|
||||
# S3; ``sagemaker_base_url`` redirects all SageMaker traffic;
|
||||
# ``deployment_url`` redirects SAP deployments.
|
||||
"s3_endpoint_url",
|
||||
"sagemaker_base_url",
|
||||
"deployment_url",
|
||||
)
|
||||
|
||||
|
||||
def _check_banned_params(
|
||||
body: dict,
|
||||
general_settings: dict,
|
||||
llm_router: Optional[Router],
|
||||
model: str,
|
||||
) -> None:
|
||||
"""Raise ``ValueError`` if ``body`` carries a banned param without admin opt-in.
|
||||
|
||||
Shared between the root-level check and the nested-config check so a
|
||||
new banned param only needs to be added in one place.
|
||||
"""
|
||||
for param in _BANNED_REQUEST_BODY_PARAMS:
|
||||
if param not in body:
|
||||
continue
|
||||
if general_settings.get("allow_client_side_credentials") is True:
|
||||
return
|
||||
if (
|
||||
_allow_model_level_clientside_configurable_parameters(
|
||||
model=model,
|
||||
param=param,
|
||||
request_body_value=body[param],
|
||||
llm_router=llm_router,
|
||||
)
|
||||
is True
|
||||
):
|
||||
return
|
||||
raise ValueError(
|
||||
f"Rejected Request: {param} is not allowed in request body. "
|
||||
"Clientside passthrough requires explicit admin opt-in via "
|
||||
"either `general_settings.allow_client_side_credentials = true` "
|
||||
"(proxy-wide) or `configurable_clientside_auth_params` on the "
|
||||
"deployment in your proxy config.yaml. "
|
||||
"Relevant Issue: https://huntr.com/bounties/4001e1a2-7b7a-4776-a3ae-e6692ec3d997",
|
||||
)
|
||||
|
||||
|
||||
def is_request_body_safe(
|
||||
request_body: dict, general_settings: dict, llm_router: Optional[Router], model: str
|
||||
) -> bool:
|
||||
@@ -175,96 +250,34 @@ def is_request_body_safe(
|
||||
|
||||
A malicious user can set the api_base to their own domain and invoke POST /chat/completions to intercept and steal the OpenAI API key.
|
||||
Relevant issue: https://huntr.com/bounties/4001e1a2-7b7a-4776-a3ae-e6692ec3d997
|
||||
|
||||
The blocklist is enforced unconditionally. Legitimate clientside
|
||||
credential / endpoint passthrough goes through one of the two
|
||||
explicit admin opt-ins (``general_settings.allow_client_side_credentials``
|
||||
proxy-wide or ``configurable_clientside_auth_params`` per deployment).
|
||||
Historically there was a third, *implicit*, *caller-controlled* path:
|
||||
``check_complete_credentials`` returned True when the caller supplied
|
||||
any non-empty ``api_key``, which made the entire blocklist a no-op.
|
||||
That bypass turned every missing entry on the blocklist into an
|
||||
exploitable SSRF / credential-exfil hole — see GHSA-jh89-88fc-qrfp,
|
||||
GHSA-3frq-6r6h-7j64, and the chain of veria-admin findings (Dv_m860l,
|
||||
b_yRJeQ5, stN90yjP, LBlyOAc8, U2TD78kg). Removed: the blocklist now
|
||||
has a single, predictable failure mode for missing entries (a 400),
|
||||
not a credential leak.
|
||||
|
||||
Iterative single-level descent into ``_NESTED_CONFIG_KEYS`` (rather
|
||||
than recursion) covers nested-config attacks like Milvus's
|
||||
``litellm_embedding_config.api_base`` (VERIA-6) without exposing a
|
||||
recursion-depth DoS surface.
|
||||
"""
|
||||
banned_params = [
|
||||
"api_base",
|
||||
"base_url",
|
||||
"user_config",
|
||||
"aws_sts_endpoint",
|
||||
"aws_web_identity_token",
|
||||
"aws_role_name",
|
||||
"vertex_credentials",
|
||||
# Endpoint-targeting fields that retarget the outbound request or
|
||||
# an observability callback. An attacker-controlled value either
|
||||
# exfiltrates the request payload (incl. messages + admin-set
|
||||
# tokens) to the attacker's host, or coerces the proxy into
|
||||
# authenticating against the attacker's host with admin secrets.
|
||||
"aws_bedrock_runtime_endpoint",
|
||||
"langsmith_base_url",
|
||||
"langfuse_host",
|
||||
"posthog_host",
|
||||
"braintrust_host",
|
||||
"slack_webhook_url",
|
||||
# Provider-specific endpoint overrides that flow into the outbound
|
||||
# request via ``optional_params``. Same threat as ``api_base``:
|
||||
# ``s3_endpoint_url`` redirects Bedrock file uploads to attacker
|
||||
# S3; ``sagemaker_base_url`` redirects all SageMaker traffic;
|
||||
# ``deployment_url`` redirects SAP deployments.
|
||||
"s3_endpoint_url",
|
||||
"sagemaker_base_url",
|
||||
"deployment_url",
|
||||
]
|
||||
|
||||
# The blocklist is enforced unconditionally. Legitimate clientside
|
||||
# credential / endpoint passthrough goes through one of the two
|
||||
# explicit admin opt-ins (``general_settings.allow_client_side_credentials``
|
||||
# proxy-wide or ``configurable_clientside_auth_params`` per deployment).
|
||||
# Historically there was a third, *implicit*, *caller-controlled* path:
|
||||
# ``check_complete_credentials`` returned True when the caller supplied
|
||||
# any non-empty ``api_key``, which made the entire blocklist a no-op.
|
||||
# That bypass turned every missing entry on the blocklist into an
|
||||
# exploitable SSRF / credential-exfil hole — see GHSA-jh89-88fc-qrfp,
|
||||
# GHSA-3frq-6r6h-7j64, and the chain of veria-admin findings (Dv_m860l,
|
||||
# b_yRJeQ5, stN90yjP, LBlyOAc8, U2TD78kg). Removed: the blocklist now
|
||||
# has a single, predictable failure mode for missing entries (a 400),
|
||||
# not a credential leak.
|
||||
for param in banned_params:
|
||||
if param in request_body:
|
||||
if general_settings.get("allow_client_side_credentials") is True:
|
||||
return True
|
||||
elif (
|
||||
_allow_model_level_clientside_configurable_parameters(
|
||||
model=model,
|
||||
param=param,
|
||||
request_body_value=request_body[param],
|
||||
llm_router=llm_router,
|
||||
)
|
||||
is True
|
||||
):
|
||||
return True
|
||||
raise ValueError(
|
||||
f"Rejected Request: {param} is not allowed in request body. "
|
||||
"Clientside passthrough requires explicit admin opt-in via "
|
||||
"either `general_settings.allow_client_side_credentials = true` "
|
||||
"(proxy-wide) or `configurable_clientside_auth_params` on the "
|
||||
"deployment in your proxy config.yaml. "
|
||||
"Relevant Issue: https://huntr.com/bounties/4001e1a2-7b7a-4776-a3ae-e6692ec3d997",
|
||||
)
|
||||
|
||||
# Recurse into nested config dicts whose values get unpacked as
|
||||
# ``**kwargs`` into outbound API calls — same SSRF / credential
|
||||
# exfil surface as the root, but historically not covered by this
|
||||
# banned-param check. VERIA-6.
|
||||
_check_banned_params(request_body, general_settings, llm_router, model)
|
||||
for nested_key in _NESTED_CONFIG_KEYS:
|
||||
nested = request_body.get(nested_key)
|
||||
if isinstance(nested, dict):
|
||||
is_request_body_safe(
|
||||
request_body=nested,
|
||||
general_settings=general_settings,
|
||||
llm_router=llm_router,
|
||||
model=model,
|
||||
)
|
||||
|
||||
_check_banned_params(nested, general_settings, llm_router, model)
|
||||
return True
|
||||
|
||||
|
||||
# Config dicts whose entries are spread as ``**dict`` into outbound LLM
|
||||
# API calls. ``litellm_embedding_config`` is consumed by the Milvus
|
||||
# vector store transformer; future nested-config keys with the same
|
||||
# threat shape should be added here.
|
||||
_NESTED_CONFIG_KEYS: Tuple[str, ...] = ("litellm_embedding_config",)
|
||||
|
||||
|
||||
async def pre_db_read_auth_checks(
|
||||
request: Request,
|
||||
request_data: dict,
|
||||
|
||||
@@ -452,3 +452,18 @@ def test_assert_same_origin_case_insensitive_host():
|
||||
assert_same_origin(
|
||||
"https://API.example.com/poll", "https://api.example.com/generate"
|
||||
)
|
||||
|
||||
|
||||
def test_assert_same_origin_error_message_does_not_leak_hostnames():
|
||||
"""Greptile P2: in the SSRF threat model the caller is the attacker.
|
||||
The error message must not echo the operator's expected host or the
|
||||
attacker-supplied candidate host back to the caller — only identify
|
||||
*which* component mismatched."""
|
||||
with pytest.raises(SSRFError) as exc:
|
||||
assert_same_origin(
|
||||
"https://attacker.example.com:1234/poll",
|
||||
"https://api.internal-corp.example/generate",
|
||||
)
|
||||
detail = str(exc.value)
|
||||
assert "attacker.example.com" not in detail
|
||||
assert "api.internal-corp.example" not in detail
|
||||
|
||||
@@ -1064,3 +1064,29 @@ class TestIsRequestBodySafeNestedConfig:
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
def test_deeply_nested_config_does_not_recurse(self):
|
||||
"""Greptile P1: ``is_request_body_safe`` is iterative single-level —
|
||||
a deeply-nested ``litellm_embedding_config`` cannot exhaust the
|
||||
Python call stack to trigger a 500 ``RecursionError``. Build a
|
||||
body 1000 levels deep; the validator must complete in O(1)
|
||||
descent."""
|
||||
body = {"litellm_embedding_config": {}}
|
||||
cur = body["litellm_embedding_config"]
|
||||
for _ in range(1000):
|
||||
cur["litellm_embedding_config"] = {}
|
||||
cur = cur["litellm_embedding_config"]
|
||||
# Banned param at the deepest level shouldn't be reached — single
|
||||
# level only.
|
||||
cur["api_base"] = "https://attacker.example.com"
|
||||
|
||||
# No exception raised: deeper levels aren't checked.
|
||||
assert (
|
||||
is_request_body_safe(
|
||||
request_body=body,
|
||||
general_settings={},
|
||||
llm_router=None,
|
||||
model="x",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user