Skip to content

fix(security): prevent Zip Slip directory traversal in skills hub downloads - #4996

Closed
Xowiek wants to merge 1 commit into
NousResearch:mainfrom
Xowiek:fix/security-zip-slip-skills-hub
Closed

fix(security): prevent Zip Slip directory traversal in skills hub downloads#4996
Xowiek wants to merge 1 commit into
NousResearch:mainfrom
Xowiek:fix/security-zip-slip-skills-hub

Conversation

@Xowiek

@Xowiek Xowiek commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

Description

🐛 The Bug

A critical Zip Slip (Directory Traversal) security vulnerability was identified in the tools/skills_hub.py module, specifically within the ClawHubSource._download_zip method.

The previous logic allowed a maliciously crafted ZIP archive to perform directory traversal using paths like ../../evil.txt or absolute paths like /tmp/evil.txt. This could potentially allow an attacker to write or overwrite arbitrary files outside the intended skill extraction directory during the download process.

🛠️ The Fix

Implemented a robust, platform-independent boundary check using pathlib to enforce strict extraction constraints:

  • Absolute Path Rejection: Explicitly rejects any ZIP member attempting to use an absolute path natively.
  • Strict Boundary Enforcement: Resolves the final destination using pathlib.Path.resolve() and utilizes .relative_to(target_dir) to definitively catch and block any traversal escapes.
  • Graceful Handling: Captures ValueError and safely skips any unsafe archive members without crashing the download loop.

✅ Validation

Successfully verified the fix using a targeted regression script simulating malicious ZIP payloads:

  • Confirmed normal files and nested safe paths (e.g., subdir/nested.md) are extracted correctly.
  • Verified that relative traversal paths (../../evil.txt) and absolute paths (/tmp/evil.txt) are strictly blocked.
  • Created a permanent regression test at tests/tools/test_zip_slip.py to ensure this vulnerability does not return.
  • Fix is confirmed to be fully cross-platform (Tested on Windows, remains strictly POSIX-compatible).

📋 Checklist

  • Bug fix (non-breaking change which fixes an issue)
  • Security patch (fixes a privilege/trust scope vulnerability)
  • Tested locally
  • Code adheres to the "Hermes Agent Contribution Guide" strictly (Security hardening & cross-platform compatibility)

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the security-minded contribution, @Xowiek! This is an automated hermes-sweeper review.

This fix was already shipped to main before this PR was opened.

  • Commit 37825189d (PR fix(skills): validate hub bundle paths before install #3986, merged 2026-03-30, five days before this PR) introduced _normalize_bundle_path / _validate_bundle_rel_path in tools/skills_hub.py (lines 88–122), which explicitly reject absolute paths, .. traversal components, and Windows drive-letter prefixes.
  • _download_zip already calls _validate_bundle_rel_path(info.filename) on every ZIP member (line 1856) and skips unsafe members gracefully on ValueError — exactly the behaviour this PR proposes.
  • A traversal regression test (test_quarantine_bundle_rejects_traversal_file_paths) was also added in that same commit at tests/tools/test_skills_hub.py:1160.

No additional changes are needed. Closing as already implemented.

@teknium1 teknium1 closed this Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants