🛡️ Sentinel: [CRITICAL] Fix Zip Slip path traversal vulnerability#154
🛡️ Sentinel: [CRITICAL] Fix Zip Slip path traversal vulnerability#154thirdeyenation wants to merge 1 commit into
Conversation
- Fixed a critical Zip Slip path traversal vulnerability in `helpers/skills_import.py` where `_unzip_to_temp_dir` used the unsafe `z.extractall(target)` without sanitizing path traversal attacks (e.g. `../`). - Implemented mitigation by manually iterating through `z.namelist()`, resolving each member path, and verifying it falls securely inside the intended target directory using `Path.is_relative_to()`. If an escape path is detected, it immediately throws a `ValueError`. - Added critical security learning record to `.jules/sentinel.md`. Co-authored-by: thirdeyenation <133812267+thirdeyenation@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability:
zipfile.ZipFile.extractall()is inherently vulnerable to "Zip Slip" directory traversal attacks if a malicious archive contains files with../pathing. This allows an attacker to extract payload files outside the temporary directory to arbitrary locations on the system where the application has write permissions (such as overwriting config files or sensitive logic).🎯 Impact: This allows arbitrary file write on the filesystem, which could lead to Remote Code Execution (RCE), data corruption, or denial-of-service depending on the permissions and paths overwritten.
🔧 Fix: Replaced
extractallwith a secure iterator overz.namelist(). Themember_pathis explicitly resolved to absolute path, and then verified to be relative to the resolvedtargetbase directory viamember_path.is_relative_to(). If a path escapes, the extraction is halted with aValueError.✅ Verification: An archive containing the file
../escaped.txtwas validated against this mitigation. When attempted on the patched_unzip_to_temp_dir(), aValueErroris successfully thrown and the malicious payload is thwarted. Tested Python test suite for zero regressions.PR created automatically by Jules for task 6524962176068506697 started by @thirdeyenation