mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-07-02 13:48:09 +00:00
Fix symlink path traversal in skill archive extraction (#6235)
* Fix symlink path traversal in skill archive extraction `_safe_extractall` (the Python < 3.12 fallback used by `crewai skills` archive unpacking) validated each member's *name* against the destination but never validated symlink/hardlink *targets*. A malicious skill tarball could plant a symlink escaping the destination (e.g. `link -> /home/user/.ssh`) followed by a regular member written through it (`link/authorized_keys`), escaping `dest` even though every member name resolves inside it — the classic symlink-extraction traversal. The 3.12+ path (`extractall(..., filter="data")`) already blocks this; the fallback now mirrors it by rejecting absolute link targets and any link target that resolves outside the destination directory. Adds regression tests covering absolute and relative escaping symlinks plus benign in-tree symlinks and ordinary archives. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Harden skill cache archive extraction * Reject special skill archive members --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -378,12 +378,40 @@ class SkillCommand(BaseCommand, PlusAPIMixin):
|
||||
|
||||
|
||||
def _safe_extractall(tf: tarfile.TarFile, dest: Path) -> None:
|
||||
"""Path-traversal-safe extraction for Python < 3.12."""
|
||||
"""Path-traversal-safe extraction for Python versions without tar filters.
|
||||
|
||||
Validates both the member's own path and, for symlink/hardlink members,
|
||||
the link target. Without the link-target check a malicious archive can
|
||||
plant a symlink that escapes ``dest`` (e.g. ``link -> /home/user/.ssh``)
|
||||
followed by a regular member written *through* that link
|
||||
(``link/authorized_keys``), escaping ``dest`` even though every member
|
||||
name resolves inside it. This mirrors the protection that
|
||||
``tarfile.extractall(..., filter="data")`` provides when available.
|
||||
"""
|
||||
dest_resolved = dest.resolve()
|
||||
for member in tf.getmembers():
|
||||
member_path = (dest / member.name).resolve()
|
||||
if not member_path.is_relative_to(dest_resolved):
|
||||
raise ValueError(f"Blocked path traversal attempt: {member.name!r}")
|
||||
if not (member.isfile() or member.isdir() or member.issym() or member.islnk()):
|
||||
raise ValueError(f"Blocked unsupported tar member: {member.name!r}")
|
||||
if member.issym() or member.islnk():
|
||||
link_target = member.linkname
|
||||
# Absolute link targets always escape the destination.
|
||||
if os.path.isabs(link_target):
|
||||
raise ValueError(
|
||||
f"Blocked link target escaping destination: "
|
||||
f"{member.name!r} -> {link_target!r}"
|
||||
)
|
||||
# Hardlink names are relative to the archive root; symlink
|
||||
# targets are relative to the member's own directory.
|
||||
anchor = dest if member.islnk() else (dest / member.name).parent
|
||||
resolved_target = (anchor / link_target).resolve()
|
||||
if not resolved_target.is_relative_to(dest_resolved):
|
||||
raise ValueError(
|
||||
f"Blocked link target escaping destination: "
|
||||
f"{member.name!r} -> {link_target!r}"
|
||||
)
|
||||
tf.extractall(dest) # noqa: S202
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user