mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-08-10 08:21:54 +00:00
fix: harden interpolated output_file path validation
Validate the fully interpolated path so adjacent placeholders cannot concatenate into traversal, and reject Windows drive/root-relative input values that bypass is_absolute(). Co-authored-by: Rip&Tear <theCyberTech@users.noreply.github.com>
This commit is contained in:
@@ -559,6 +559,22 @@ class Task(BaseModel):
|
||||
return value[1:]
|
||||
return value
|
||||
|
||||
@staticmethod
|
||||
def _is_unsafe_absolute_output_path(value: str) -> bool:
|
||||
"""Return True if ``value`` is an absolute or drive/root-relative path.
|
||||
|
||||
Includes Windows drive-qualified relative paths (``C:foo``) and
|
||||
root-relative paths (``\\Windows\\...``) that ``is_absolute()`` misses
|
||||
but that still escape a relative working directory on Windows.
|
||||
"""
|
||||
windows_path = PureWindowsPath(value)
|
||||
return bool(
|
||||
PurePosixPath(value).is_absolute()
|
||||
or windows_path.is_absolute()
|
||||
or windows_path.drive
|
||||
or windows_path.root
|
||||
)
|
||||
|
||||
def _validate_output_file_input_values(
|
||||
self, inputs: dict[str, str | int | float | dict[str, Any] | list[Any]]
|
||||
) -> None:
|
||||
@@ -583,28 +599,65 @@ class Task(BaseModel):
|
||||
value = str(inputs[var])
|
||||
if ".." in value:
|
||||
raise ValueError(
|
||||
f"Invalid value for output_file variable '{var}': path "
|
||||
f"Invalid value for output_file variable '{var}': Path "
|
||||
"traversal sequences ('..') are not allowed"
|
||||
)
|
||||
if value.startswith(("~", "$")):
|
||||
raise ValueError(
|
||||
f"Invalid value for output_file variable '{var}': shell "
|
||||
f"Invalid value for output_file variable '{var}': Shell "
|
||||
"expansion characters are not allowed"
|
||||
)
|
||||
if any(char in value for char in ["|", ">", "<", "&", ";"]):
|
||||
raise ValueError(
|
||||
f"Invalid value for output_file variable '{var}': shell "
|
||||
f"Invalid value for output_file variable '{var}': Shell "
|
||||
"special characters are not allowed"
|
||||
)
|
||||
if (
|
||||
PurePosixPath(value).is_absolute()
|
||||
or PureWindowsPath(value).is_absolute()
|
||||
):
|
||||
if self._is_unsafe_absolute_output_path(value):
|
||||
raise ValueError(
|
||||
f"Invalid value for output_file variable '{var}': absolute "
|
||||
f"Invalid value for output_file variable '{var}': Absolute "
|
||||
"paths are not allowed"
|
||||
)
|
||||
|
||||
def _validate_interpolated_output_file(self, interpolated: str) -> None:
|
||||
"""Reject a fully interpolated path that escapes the trusted template.
|
||||
|
||||
Per-value checks miss hazards formed by concatenating adjacent
|
||||
placeholders (for example ``{a}{b}`` with ``a='.'`` and ``b='.'``).
|
||||
The developer-authored template may include an absolute base directory;
|
||||
that remains allowed when the interpolated result stays absolute for the
|
||||
same reason. A relative template must not become absolute, and no
|
||||
interpolated path may introduce traversal or shell metacharacters.
|
||||
"""
|
||||
template = self._original_output_file
|
||||
if not template:
|
||||
return
|
||||
|
||||
if ".." in interpolated:
|
||||
raise ValueError(
|
||||
"Path traversal attempts are not allowed in output_file paths"
|
||||
)
|
||||
if interpolated.startswith(("~", "$")):
|
||||
raise ValueError(
|
||||
"Shell expansion characters are not allowed in output_file paths"
|
||||
)
|
||||
if any(char in interpolated for char in ["|", ">", "<", "&", ";"]):
|
||||
raise ValueError(
|
||||
"Shell special characters are not allowed in output_file paths"
|
||||
)
|
||||
|
||||
dummy_filled = template
|
||||
for var in (part.split("}")[0] for part in template.split("{")[1:]):
|
||||
dummy_filled = dummy_filled.replace("{" + var + "}", "_x_")
|
||||
|
||||
template_is_absolute = self._is_unsafe_absolute_output_path(dummy_filled)
|
||||
if (
|
||||
self._is_unsafe_absolute_output_path(interpolated)
|
||||
and not template_is_absolute
|
||||
):
|
||||
raise ValueError(
|
||||
"Absolute paths are not allowed in interpolated output_file paths"
|
||||
)
|
||||
|
||||
@model_validator(mode="after")
|
||||
def set_attributes_based_on_config(self) -> Task:
|
||||
"""Set attributes based on the agent configuration."""
|
||||
@@ -1167,14 +1220,19 @@ Follow these guidelines:
|
||||
# kickoff inputs. The developer-authored template (including any
|
||||
# absolute base directory) is trusted, but an injected value must
|
||||
# not introduce path traversal, an absolute path, or shell
|
||||
# expansion that would escape the intended location.
|
||||
# expansion that would escape the intended location. Per-value
|
||||
# checks run first; the fully interpolated path is then checked so
|
||||
# concatenated placeholders cannot form a hazard the individual
|
||||
# values alone would miss.
|
||||
self._validate_output_file_input_values(inputs)
|
||||
try:
|
||||
self.output_file = interpolate_only(
|
||||
interpolated_output_file = interpolate_only(
|
||||
input_string=self._original_output_file, inputs=inputs
|
||||
)
|
||||
except (KeyError, ValueError) as e:
|
||||
raise ValueError(f"Error interpolating output_file path: {e!s}") from e
|
||||
self._validate_interpolated_output_file(interpolated_output_file)
|
||||
self.output_file = interpolated_output_file
|
||||
|
||||
if inputs.get("crew_chat_messages"):
|
||||
conversation_instruction = I18N_DEFAULT.slice(
|
||||
|
||||
@@ -935,11 +935,16 @@ def test_interpolate_inputs(tmp_path):
|
||||
@pytest.mark.parametrize(
|
||||
("template", "malicious_inputs", "expected_error"),
|
||||
[
|
||||
("reports/{name}.md", {"name": "../../../../tmp/pwn"}, "path traversal"),
|
||||
("{p}", {"p": "/tmp/abs_pwn"}, "absolute paths"),
|
||||
("{p}", {"p": "~/.bashrc"}, "shell expansion"),
|
||||
("{p}", {"p": "x;rm -rf /"}, "shell special characters"),
|
||||
("{p}", {"p": r"C:\Windows\evil"}, "absolute paths"),
|
||||
("reports/{name}.md", {"name": "../../../../tmp/pwn"}, "Path traversal"),
|
||||
("{p}", {"p": "/tmp/abs_pwn"}, "Absolute paths"),
|
||||
("{p}", {"p": "~/.bashrc"}, "Shell expansion"),
|
||||
("{p}", {"p": "x;rm -rf /"}, "Shell special characters"),
|
||||
("{p}", {"p": r"C:\Windows\evil"}, "Absolute paths"),
|
||||
# Drive-qualified relative path: not absolute() but escapes on Windows.
|
||||
("{p}", {"p": r"C:Windows\evil"}, "Absolute paths"),
|
||||
# Adjacent placeholders can concatenate into ".." even when each value
|
||||
# alone looks safe.
|
||||
("{a}{b}", {"a": ".", "b": "."}, "Path traversal"),
|
||||
],
|
||||
)
|
||||
def test_interpolate_output_file_rejects_unsafe_inputs(
|
||||
@@ -947,7 +952,7 @@ def test_interpolate_output_file_rejects_unsafe_inputs(
|
||||
):
|
||||
"""Untrusted inputs must not escape the output_file path via interpolation."""
|
||||
task = Task(
|
||||
description="do {p} {name}".replace("{p}", "x").replace("{name}", "x"),
|
||||
description="d",
|
||||
expected_output="e",
|
||||
output_file=template,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user