diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000000..15a506abf3 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2026-05-22 - Zip Slip in skills_import.py +**Vulnerability:** A path traversal vulnerability (Zip Slip) was found in `helpers/skills_import.py` where `zipfile.ZipFile.extractall()` was called without explicit validation of the member paths, allowing a malicious archive to write outside the intended extraction directory. +**Learning:** Even if modern Python versions of `extractall()` include some mitigations against basic `../` traversal, explicit path validation (checking `os.path.realpath` against the target directory, or using `str.startswith()` with absolute paths) is the most robust and secure pattern to follow. The codebase already implements this correctly in other places (like plugin installation and validation). +**Prevention:** Always validate each member of a ZIP archive in `z.namelist()` using `files.is_in_dir()` or a similar robust path containment check before extracting it. Do not rely solely on the default behavior of `extractall()`. diff --git a/helpers/skills_import.py b/helpers/skills_import.py index c73ffa55de..0436deccc9 100644 --- a/helpers/skills_import.py +++ b/helpers/skills_import.py @@ -94,6 +94,10 @@ def _unzip_to_temp_dir(zip_path: Path) -> Path: target.mkdir(parents=True, exist_ok=True) with zipfile.ZipFile(zip_path, "r") as z: + for member in z.namelist(): + member_path = os.path.realpath(os.path.join(target, member)) + if not files.is_in_dir(member_path, str(target)): + raise ValueError(f"Unsafe path in archive: {member}") z.extractall(target) # If zip contains a single top-level folder, treat that as the root