Skip to content

fix(skills): prevent path traversal in LocalSkillSource via normalize + boundary check#1311

Open
herdiyana256 wants to merge 1 commit into
google:mainfrom
herdiyana256:fix/path-traversal-local-skill-source
Open

fix(skills): prevent path traversal in LocalSkillSource via normalize + boundary check#1311
herdiyana256 wants to merge 1 commit into
google:mainfrom
herdiyana256:fix/path-traversal-local-skill-source

Conversation

@herdiyana256

Copy link
Copy Markdown

Summary

LoadSkillResourceTool validates resourcePath with a string prefix check
(startsWith("references/"), startsWith("assets/"), startsWith("scripts/")),
but this check is trivially bypassed:
"references/../../../../etc/passwd".startsWith("references/") == true // bypassed!

LocalSkillSource.findResourcePath() then resolves the raw path against
skillsBasePath without calling Path.normalize() or asserting a boundary,
so the OS resolves .. components at Files.exists() time — resulting in
arbitrary file reads outside skillsBasePath.

The same issue exists in listResources() for the resourceDirectory parameter.

Fix

In LocalSkillSource.findResourcePath() and listResources():

  1. Call .normalize() on the resolved path
  2. Assert file.startsWith(base) — reject if the resolved path escapes the skill directory

The correct enforcement point is the filesystem layer after resolution,
not a string prefix check on raw user input.

Files Changed

  • core/src/main/java/com/google/adk/skills/LocalSkillSource.java
    • findResourcePath(): normalize + boundary check
    • listResources(): normalize + boundary check on resourceDirectory

Testing

# Path traversal is now blocked:
# "references/../../../../etc/passwd" → SkillSourceException: Path traversal detected

javac PathTraversalPoC.java && java PathTraversalPoC
# [ESCAPE] Escapes skillsBasePath: true  ← without fix
# After fix: Single.error(SkillSourceException) returned before Files.exists()

References
: CWE-22: Improper Limitation of a Pathname to a Restricted Directory
: Related VRP: Issue 528977579

The startsWith("references/") prefix check in LoadSkillResourceTool is
bypassed by payloads like "references/../../../../etc/passwd" — the
string prefix matches while the resolved path escapes skillsBasePath.

Fix by calling Path.normalize() on the resolved path and asserting that
it still starts with the expected base directory before any filesystem
access in both findResourcePath() and listResources().

Fixes CWE-22 (Path Traversal).
@herdiyana256 herdiyana256 force-pushed the fix/path-traversal-local-skill-source branch from f0a8a8c to 30ba017 Compare June 28, 2026 16:16
@hemasekhar-p hemasekhar-p self-assigned this Jun 29, 2026
@hemasekhar-p

Copy link
Copy Markdown
Contributor

Hi @herdiyana256, thank you for your contribution! We appreciate you taking the time to submit this pull request. Could you please include the corresponding unit tests to verify your changes and i noticed some inconsistencies in the code formatting. Could you please take a look and address those issues?

@hemasekhar-p hemasekhar-p added the waiting on reporter Waiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale. label Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting on reporter Waiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants