mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
fix(guardrails): add directory jail to content_filter file path resolution (CWE-022)
_resolve_category_file_path() accepted a category_file path from guardrail config and opened it without verifying it stays inside the categories/ directory. An admin who can write guardrail YAML config could set category_file: "../../../../etc/passwd" to read any file the proxy process can access. - Added _assert_within_categories_dir() static method using os.path.realpath() + os.path.commonpath() to jail every resolved path inside categories/. - Changed _resolve_category_file_path from @staticmethod to instance method and applied the jail check on all resolution branches (absolute, relative, module-relative, suffix-stripped) including the final fallback so traversal attempts are rejected regardless of CWD or file existence. - Added re.match(r'^[a-zA-Z0-9_\-]+$', category_name) guard in _load_categories to block traversal via the category_name field itself.
This commit is contained in:
parent
cff3e0b75e
commit
f176966be8
2 changed files with 139 additions and 6 deletions
|
|
@ -328,7 +328,22 @@ class ContentFilterGuardrail(CustomGuardrail):
|
|||
return result
|
||||
|
||||
@staticmethod
|
||||
def _resolve_category_file_path(file_path: str) -> str:
|
||||
def _assert_within_categories_dir(path: str, categories_dir: str) -> None:
|
||||
"""Raise ValueError if path escapes the categories directory."""
|
||||
resolved = os.path.realpath(path)
|
||||
allowed = os.path.realpath(categories_dir)
|
||||
try:
|
||||
if os.path.commonpath([resolved, allowed]) != allowed:
|
||||
raise ValueError(
|
||||
f"Category file path '{path}' is outside the allowed "
|
||||
f"categories directory '{categories_dir}'"
|
||||
)
|
||||
except ValueError as exc:
|
||||
raise ValueError(
|
||||
f"Category file path '{path}' is outside the allowed categories directory"
|
||||
) from exc
|
||||
|
||||
def _resolve_category_file_path(self, file_path: str) -> str:
|
||||
"""
|
||||
Resolve a category file path that may be relative.
|
||||
|
||||
|
|
@ -339,12 +354,10 @@ class ContentFilterGuardrail(CustomGuardrail):
|
|||
file isn't found.
|
||||
|
||||
Resolution order:
|
||||
1. Return as-is if absolute or already exists.
|
||||
2. Try joining the full path relative to this module's directory.
|
||||
1. Return as-is if absolute or already exists (jailed to categories/).
|
||||
2. Try joining the full path relative to this module's directory (jailed).
|
||||
3. Progressively strip leading path components and try each suffix
|
||||
relative to this module's directory (handles paths like
|
||||
"litellm/proxy/.../policy_templates/file.yaml" by finding the
|
||||
"policy_templates/file.yaml" suffix that exists).
|
||||
relative to this module's directory (jailed).
|
||||
|
||||
Args:
|
||||
file_path: The file path to resolve (absolute or relative).
|
||||
|
|
@ -352,8 +365,14 @@ class ContentFilterGuardrail(CustomGuardrail):
|
|||
Returns:
|
||||
The resolved absolute-ish path, or the original path if
|
||||
resolution fails (caller should check existence).
|
||||
|
||||
Raises:
|
||||
ValueError: If the resolved path escapes the categories directory.
|
||||
"""
|
||||
categories_dir = os.path.join(os.path.dirname(__file__), "categories")
|
||||
|
||||
if os.path.isabs(file_path) or os.path.exists(file_path):
|
||||
self._assert_within_categories_dir(file_path, categories_dir)
|
||||
return file_path
|
||||
|
||||
module_dir = os.path.dirname(__file__)
|
||||
|
|
@ -361,6 +380,7 @@ class ContentFilterGuardrail(CustomGuardrail):
|
|||
# Try the full relative path joined to the module directory
|
||||
candidate = os.path.join(module_dir, file_path)
|
||||
if os.path.exists(candidate):
|
||||
self._assert_within_categories_dir(candidate, categories_dir)
|
||||
return candidate
|
||||
|
||||
# Progressively strip leading components to find a matching suffix
|
||||
|
|
@ -369,8 +389,15 @@ class ContentFilterGuardrail(CustomGuardrail):
|
|||
suffix = os.path.join(*parts[i:])
|
||||
candidate = os.path.join(module_dir, suffix)
|
||||
if os.path.exists(candidate):
|
||||
self._assert_within_categories_dir(candidate, categories_dir)
|
||||
return candidate
|
||||
|
||||
# File not found via any resolution strategy — jail the module-relative
|
||||
# path anyway to reject traversal attempts (e.g. "../../../../etc/passwd")
|
||||
# regardless of CWD or whether the target file exists.
|
||||
self._assert_within_categories_dir(
|
||||
os.path.join(module_dir, file_path), categories_dir
|
||||
)
|
||||
return file_path
|
||||
|
||||
def _load_categories(self, categories: List[ContentFilterCategoryConfig]) -> None:
|
||||
|
|
@ -395,6 +422,13 @@ class ContentFilterGuardrail(CustomGuardrail):
|
|||
)
|
||||
continue
|
||||
|
||||
# Prevent path traversal via category_name (e.g. "../../etc/passwd")
|
||||
if not re.match(r"^[a-zA-Z0-9_\-]+$", category_name):
|
||||
verbose_proxy_logger.warning(
|
||||
f"Category name '{category_name}' contains invalid characters, skipping"
|
||||
)
|
||||
continue
|
||||
|
||||
enabled = cat_config.get("enabled", True)
|
||||
action = cat_config.get("action")
|
||||
severity_threshold = (
|
||||
|
|
|
|||
|
|
@ -0,0 +1,99 @@
|
|||
import os
|
||||
import pytest
|
||||
|
||||
|
||||
class TestContentFilterPathTraversal:
|
||||
"""Tests that _resolve_category_file_path rejects path traversal."""
|
||||
|
||||
def _get_guardrail(self):
|
||||
from litellm.proxy.guardrails.guardrail_hooks.litellm_content_filter.content_filter import (
|
||||
ContentFilterGuardrail,
|
||||
)
|
||||
|
||||
return ContentFilterGuardrail.__new__(ContentFilterGuardrail)
|
||||
|
||||
def test_traversal_via_relative_dotdot_raises(self):
|
||||
guardrail = self._get_guardrail()
|
||||
with pytest.raises(ValueError, match="outside the allowed categories"):
|
||||
guardrail._resolve_category_file_path("../../../../etc/passwd")
|
||||
|
||||
def test_traversal_via_absolute_path_raises(self):
|
||||
guardrail = self._get_guardrail()
|
||||
with pytest.raises(ValueError, match="outside the allowed categories"):
|
||||
guardrail._resolve_category_file_path("/etc/passwd")
|
||||
|
||||
def test_valid_category_file_inside_categories_dir_allowed(self):
|
||||
guardrail = self._get_guardrail()
|
||||
categories_dir = os.path.join(
|
||||
os.path.dirname(
|
||||
__import__(
|
||||
"litellm.proxy.guardrails.guardrail_hooks.litellm_content_filter.content_filter",
|
||||
fromlist=["content_filter"],
|
||||
).__file__
|
||||
),
|
||||
"categories",
|
||||
)
|
||||
valid_file = os.path.join(categories_dir, "harmful_self_harm.yaml")
|
||||
if os.path.exists(valid_file):
|
||||
result = guardrail._resolve_category_file_path(valid_file)
|
||||
assert result == valid_file
|
||||
|
||||
def test_invalid_category_name_skipped(self):
|
||||
from litellm.proxy.guardrails.guardrail_hooks.litellm_content_filter.content_filter import (
|
||||
ContentFilterGuardrail,
|
||||
)
|
||||
|
||||
guardrail = ContentFilterGuardrail.__new__(ContentFilterGuardrail)
|
||||
guardrail.loaded_categories = {}
|
||||
guardrail.severity_threshold = "medium"
|
||||
guardrail.category_keywords = {}
|
||||
guardrail.always_block_category_keywords = {}
|
||||
guardrail.conditional_categories = {}
|
||||
# category name with path traversal chars must be skipped, not crash
|
||||
guardrail._load_categories([{"category": "../../etc/passwd", "enabled": True}])
|
||||
assert "../../etc/passwd" not in guardrail.loaded_categories
|
||||
|
||||
def test_category_name_with_slash_skipped(self):
|
||||
from litellm.proxy.guardrails.guardrail_hooks.litellm_content_filter.content_filter import (
|
||||
ContentFilterGuardrail,
|
||||
)
|
||||
|
||||
guardrail = ContentFilterGuardrail.__new__(ContentFilterGuardrail)
|
||||
guardrail.loaded_categories = {}
|
||||
guardrail.severity_threshold = "medium"
|
||||
guardrail.category_keywords = {}
|
||||
guardrail.always_block_category_keywords = {}
|
||||
guardrail.conditional_categories = {}
|
||||
guardrail._load_categories(
|
||||
[{"category": "foo/../../etc/passwd", "enabled": True}]
|
||||
)
|
||||
assert "foo/../../etc/passwd" not in guardrail.loaded_categories
|
||||
|
||||
def test_assert_within_categories_dir_blocks_parent_traversal(self):
|
||||
from litellm.proxy.guardrails.guardrail_hooks.litellm_content_filter.content_filter import (
|
||||
ContentFilterGuardrail,
|
||||
)
|
||||
|
||||
categories_dir = os.path.join(
|
||||
os.path.dirname(
|
||||
__import__(
|
||||
"litellm.proxy.guardrails.guardrail_hooks.litellm_content_filter.content_filter",
|
||||
fromlist=["content_filter"],
|
||||
).__file__
|
||||
),
|
||||
"categories",
|
||||
)
|
||||
with pytest.raises(ValueError, match="outside the allowed categories"):
|
||||
ContentFilterGuardrail._assert_within_categories_dir(
|
||||
"/etc/passwd", categories_dir
|
||||
)
|
||||
|
||||
def test_assert_within_categories_dir_allows_valid_file(self, tmp_path):
|
||||
from litellm.proxy.guardrails.guardrail_hooks.litellm_content_filter.content_filter import (
|
||||
ContentFilterGuardrail,
|
||||
)
|
||||
|
||||
categories_dir = str(tmp_path)
|
||||
valid_file = str(tmp_path / "test.yaml")
|
||||
# Should not raise
|
||||
ContentFilterGuardrail._assert_within_categories_dir(valid_file, categories_dir)
|
||||
Loading…
Add table
Reference in a new issue