mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
fix(rate-limiting): close the same caller-forged metadata bypass at admission
The base rename (resolve_success_event_metadata_variable_name -> resolve_authoritative_metadata_variable_name) broke this hook's import. Fixing the import also surfaced that this hook's own admission still used get_metadata_variable_name_from_kwargs, the naive key-presence check the sibling hook's own fix just replaced: a caller forging an empty (or None) litellm_metadata alongside the real, populated metadata made admission read no tags at all, admitting past every configured limit. Confirmed exploitable via a direct repro (5/5 requests admitted against a limit of 1) before the fix, blocked after. Regression test added.
This commit is contained in:
parent
5d0eca67e8
commit
1fe4b6699d
2 changed files with 47 additions and 4 deletions
|
|
@ -47,7 +47,6 @@ from litellm._logging import verbose_proxy_logger
|
|||
from litellm.caching.dual_cache import DualCache
|
||||
from litellm.caching.in_memory_cache import InMemoryCache
|
||||
from litellm.integrations.custom_logger import CustomLogger
|
||||
from litellm.litellm_core_utils.core_helpers import get_metadata_variable_name_from_kwargs
|
||||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitError
|
||||
from litellm.proxy.hooks.parallel_request_limiter_v3 import (
|
||||
|
|
@ -112,7 +111,7 @@ from litellm.proxy.hooks.tag_rate_limits_shared import (
|
|||
policy_fingerprint as _policy_fingerprint,
|
||||
)
|
||||
from litellm.proxy.hooks.tag_rate_limits_shared import (
|
||||
resolve_success_event_metadata_variable_name as _resolve_success_event_metadata_variable_name,
|
||||
resolve_authoritative_metadata_variable_name as _resolve_authoritative_metadata_variable_name,
|
||||
)
|
||||
from litellm.proxy.utils import InternalUsageCache
|
||||
from litellm.router_strategy.tag_based_routing import (
|
||||
|
|
@ -544,7 +543,12 @@ class _PROXY_GlobalTagRateLimitsHook( # pyright: ignore[reportUnusedClass] # o
|
|||
# re-charging.
|
||||
stash: Final = _claim_stash_for_data(data)
|
||||
|
||||
metadata_variable_name: Final = get_metadata_variable_name_from_kwargs(data)
|
||||
# Not get_metadata_variable_name_from_kwargs (naive key-presence
|
||||
# check): a caller can forge an empty (or None) litellm_metadata on
|
||||
# an ordinary request to make that check pick it over the real,
|
||||
# populated metadata the proxy wrote identity/tags into, seeing no
|
||||
# tags at all and admitting past every configured limit.
|
||||
metadata_variable_name: Final = _resolve_authoritative_metadata_variable_name(data)
|
||||
tags: Final = _order_tags_for_identity_resolution(
|
||||
_get_tags_from_request_kwargs(data, metadata_variable_name=metadata_variable_name),
|
||||
data,
|
||||
|
|
@ -716,7 +720,7 @@ class _PROXY_GlobalTagRateLimitsHook( # pyright: ignore[reportUnusedClass] # o
|
|||
litellm_params_for_metadata: Final[Mapping[str, object]] = (
|
||||
litellm_params_raw if isinstance(litellm_params_raw, Mapping) else kwargs
|
||||
)
|
||||
metadata_variable_name: Final = _resolve_success_event_metadata_variable_name(litellm_params_for_metadata)
|
||||
metadata_variable_name: Final = _resolve_authoritative_metadata_variable_name(litellm_params_for_metadata)
|
||||
key_hash: Final = _extract_key_hash(litellm_params_for_metadata, metadata_variable_name)
|
||||
key_alias: Final = _extract_key_alias(litellm_params_for_metadata, metadata_variable_name)
|
||||
|
||||
|
|
|
|||
|
|
@ -1509,3 +1509,42 @@ async def test_concurrency_limit_admission_refreshes_ttl_end_to_end(time_control
|
|||
inflight_keys = [key for key in in_memory_cache.ttl_dict if key.endswith(":inflight")]
|
||||
assert len(inflight_keys) == 1
|
||||
assert in_memory_cache.ttl_dict[inflight_keys[0]] > time.time()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Admission must resolve the authoritative metadata bucket, not the naive
|
||||
# key-presence check -- veria-ai finding on the sibling model-based hook,
|
||||
# same vulnerability class in this hook's own admission call site
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_admission_ignores_a_forged_empty_litellm_metadata_key(time_controller, monkeypatch):
|
||||
"""
|
||||
get_metadata_variable_name_from_kwargs picks "litellm_metadata" whenever
|
||||
that key is merely present, regardless of its value. A caller adding an
|
||||
empty "litellm_metadata" alongside the real, populated "metadata" made
|
||||
admission read no tags at all, sailing past every configured limit.
|
||||
"""
|
||||
monkeypatch.setattr(
|
||||
litellm,
|
||||
"global_tag_rate_limits",
|
||||
{
|
||||
"request_limits": {
|
||||
"limits": [{"name": "daily", "tag_id": "end_user_id", "limit": 1, "period_seconds": 86400}]
|
||||
}
|
||||
},
|
||||
)
|
||||
hook = _make_hook(time_controller)
|
||||
poisoned = {"metadata": {"tags": ["end_user_id:u1"]}, "litellm_metadata": {}}
|
||||
|
||||
await hook.async_pre_call_hook(
|
||||
user_api_key_dict=_key(), cache=DualCache(), data={**poisoned, "litellm_call_id": "call-1"}, call_type="completion"
|
||||
)
|
||||
with pytest.raises(ProxyRateLimitError):
|
||||
await hook.async_pre_call_hook(
|
||||
user_api_key_dict=_key(),
|
||||
cache=DualCache(),
|
||||
data={**poisoned, "litellm_call_id": "call-2"},
|
||||
call_type="completion",
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue