Skip to content

Security: Fix path traversal in GCS skill extraction (Zip Slip)#5927

Open
Ashutosh0x wants to merge 1 commit into
google:mainfrom
Ashutosh0x:fix/gcs-skill-path-traversal
Open

Security: Fix path traversal in GCS skill extraction (Zip Slip)#5927
Ashutosh0x wants to merge 1 commit into
google:mainfrom
Ashutosh0x:fix/gcs-skill-path-traversal

Conversation

@Ashutosh0x
Copy link
Copy Markdown

@Ashutosh0x Ashutosh0x commented Jun 1, 2026

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_code method in skill_toolset.py generates Python code that extracts skill files into a temporary directory. The relative paths from GCS blob names are used directly with os.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 with runpy.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:

  1. Normalize the relative path with os.path.normpath()
  2. Reject paths starting with .. (parent directory traversal)
  3. Reject absolute paths via os.path.isabs()
  4. Use os.path.abspath(td) for the base directory to prevent bypasses

Testing

Unit Tests Added

Added tests/unittests/tools/test_skill_path_traversal.py with 5 tests:

Test Description
test_traversal_blocked_in_generated_code Verifies generated code contains normpath/isabs/PermissionError checks
test_safe_paths_pass_validation Verifies legitimate paths (including subdirectories) still work
test_execute_with_traversal_paths_raises End-to-end test with mock executor verifying traversal paths are blocked
test_double_dot_path_blocked Verifies ../../ traversal patterns are blocked at runtime
test_absolute_path_blocked Verifies absolute paths like /etc/passwd are blocked

Testing Plan

# Run the new path traversal tests
pytest tests/unittests/tools/test_skill_path_traversal.py -v

# Run the full skill toolset test suite to verify no regressions
pytest tests/unittests/tools/test_skill_toolset.py -v

# Run all unit tests
pytest tests/unittests/ -v

Fixes #5603

@adk-bot adk-bot added the tools [Component] This issue is related to tools label Jun 1, 2026
@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented Jun 1, 2026

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?

  • Unit Tests: Please add or update unit tests under the tests/unittests/ directory covering the directory traversal checks in both _local_environment.py and skill_toolset.py.
  • Pytest Results: Please run pytest or tox and include a summary of the passed test results.
  • Testing Plan & Logs: Please provide a more detailed testing plan along with execution logs or verification steps demonstrating that path traversal is successfully blocked.

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
@Ashutosh0x Ashutosh0x force-pushed the fix/gcs-skill-path-traversal branch from 3219e4c to cc5d670 Compare June 1, 2026 14:18
@Ashutosh0x
Copy link
Copy Markdown
Author

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:

  • Generated code contains normpath/isabs/PermissionError path traversal checks
  • Legitimate paths (including subdirectories) still work correctly
  • End-to-end execution with mock executor verifying traversal paths are blocked
  • ../../ parent directory traversal patterns are blocked
  • Absolute paths like /etc/passwd are blocked

Testing Plan:

pytest tests/unittests/tools/test_skill_path_traversal.py -v
pytest tests/unittests/tools/test_skill_toolset.py -v

Fix Details:

@Ashutosh0x
Copy link
Copy Markdown
Author

@rohityan Hi! This PR fixes the same class of vulnerability as #5603 / PR #5281 (which you are reviewing). My fix targets the _build_wrapper_code method in skill_toolset.py specifically, adding os.path.normpath() validation to prevent Zip Slip-style directory traversal via malicious GCS skill resource names.

I've included unit tests in tests/unittests/tools/test_skill_path_traversal.py (5 tests covering traversal blocking, safe path passthrough, and absolute path rejection). All CI checks are green. CLA is signed.

This is a single clean commit with fix + tests. Could you please review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools [Component] This issue is related to tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: Path Traversal in _load_skill_from_gcs_dir enables arbitrary file write (VRP #499557362)

2 participants