Security: Fix path traversal in GCS skill extraction (Zip Slip)#5927
Security: Fix path traversal in GCS skill extraction (Zip Slip)#5927Ashutosh0x wants to merge 1 commit into
Conversation
|
Response from ADK Triaging Agent Hello @Ashutosh0x, thank you for submitting this security fix! To help our reviewers process this PR more efficiently, could you please make sure the following requirements from our Contribution Guidelines are addressed?
Thank you for your valuable contribution! |
Add path normalization and validation in _build_wrapper_code to prevent directory traversal via malicious GCS skill resource names. A crafted skill resource name containing '../' could write files outside the temporary directory, potentially leading to RCE via runpy.run_path(). Changes: - Normalize relative paths with os.path.normpath() before extraction - Reject paths starting with '..' or absolute paths - Use os.path.abspath(td) for robust base directory resolution - Add unit tests covering traversal blocking and safe path passthrough Testing: - Added tests/unittests/tools/test_skill_path_traversal.py with 5 tests - Tests verify normpath/isabs/PermissionError checks in generated code - Tests verify safe paths (including subdirectories) are unaffected Fixes google#5603
3219e4c to
cc5d670
Compare
|
Hi @adk-bot, thanks for the review checklist! Here's my response: Unit Tests: Added ests/unittests/tools/test_skill_path_traversal.py with 5 tests covering:
Testing Plan: pytest tests/unittests/tools/test_skill_path_traversal.py -v
pytest tests/unittests/tools/test_skill_toolset.py -vFix Details:
|
|
@rohityan Hi! This PR fixes the same class of vulnerability as #5603 / PR #5281 (which you are reviewing). My fix targets the I've included unit tests in This is a single clean commit with fix + tests. Could you please review? |
Summary
Fix for #5603 - Validate normalized relative paths in skill extraction code to prevent directory traversal via malicious GCS skill resource names.
Problem
The
_build_wrapper_codemethod inskill_toolset.pygenerates Python code that extracts skill files into a temporary directory. The relative paths from GCS blob names are used directly withos.path.join(td, rel_path)without validation.A malicious skill resource name containing
../(e.g.,references/../../etc/cron.d/evil) would resolve outside the temporary directory, enabling arbitrary file writes. Combined withrunpy.run_path(), this creates a path-traversal-to-RCE chain (CWE-22 / Zip Slip variant).Fix
Added path normalization and validation before file extraction in the generated wrapper code:
os.path.normpath()..(parent directory traversal)os.path.isabs()os.path.abspath(td)for the base directory to prevent bypassesTesting
Unit Tests Added
Added
tests/unittests/tools/test_skill_path_traversal.pywith 5 tests:test_traversal_blocked_in_generated_codetest_safe_paths_pass_validationtest_execute_with_traversal_paths_raisestest_double_dot_path_blocked../../traversal patterns are blocked at runtimetest_absolute_path_blocked/etc/passwdare blockedTesting Plan
Fixes #5603