From c886252053934d5b9e5220a0b637dbdea34e4e8d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 24 May 2026 08:57:18 +0000 Subject: [PATCH] Fix Zip Slip in skill import Co-authored-by: thirdeyenation <133812267+thirdeyenation@users.noreply.github.com> --- .jules/sentinel.md | 8 ++++++++ helpers/skills_import.py | 5 +++++ 2 files changed, 13 insertions(+) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000000..bfc0185709 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,8 @@ +## YYYY-MM-DD - [Title] +**Vulnerability:** [What you found] +**Learning:** [Why it existed] +**Prevention:** [How to avoid next time] +## 2024-05-24 - Path Traversal in Skill Imports +**Vulnerability:** Zip Slip / Path Traversal in `helpers/skills_import.py` due to unsafe `zipfile.ZipFile.extractall()` without path containment checks. +**Learning:** `extractall()` blindly follows paths in zip files, which can include `../` to escape the extraction directory. +**Prevention:** Always iterate through `z.namelist()`, construct absolute paths, and explicitly verify they are within the target directory using `Path.is_relative_to()` before extraction. diff --git a/helpers/skills_import.py b/helpers/skills_import.py index c73ffa55de..8347d04d37 100644 --- a/helpers/skills_import.py +++ b/helpers/skills_import.py @@ -93,7 +93,12 @@ def _unzip_to_temp_dir(zip_path: Path) -> Path: target = base_tmp / f"import_{zip_path.stem}_{stamp}" target.mkdir(parents=True, exist_ok=True) + target_resolved = target.resolve() with zipfile.ZipFile(zip_path, "r") as z: + for member in z.namelist(): + member_path = (target_resolved / member).resolve() + if not member_path.is_relative_to(target_resolved): + raise ValueError(f"Unsafe path in archive: {member}") z.extractall(target) # If zip contains a single top-level folder, treat that as the root