fix(agents): prevent path traversal in AgentTool config_path resolution#5826
fix(agents): prevent path traversal in AgentTool config_path resolution#5826adilburaksen wants to merge 5 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Response from ADK Triaging Agent Hello @adilburaksen, thank you for creating this PR! We really appreciate your contribution to fixing this path traversal vulnerability. However, it looks like this PR is currently not fully following our Contribution Guidelines. Could you please address the following items:
Once these items are addressed, it will be much easier and faster for our reviewers to evaluate your PR. Thank you! |
|
I have read the CLA Documents and I hereby sign the CLA. |
Absolute config_path values were accepted unconditionally, and relative paths were joined without boundary validation, allowing traversal outside the agent directory via "../../../etc/passwd" style inputs. Fix: reject absolute paths; for relative paths, verify the normalized result stays within the parent agent's directory before loading.
c04372a to
1274b09
Compare
|
Hi @adilburaksen , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Can you please fix the failing unit tests before we can proceed with the review. |
…ixes Replace os.sep with os.path.sep in the traversal boundary check so the check works correctly when os.path is replaced with ntpath (Windows paths). os.sep is not affected by patching os.path; os.path.sep is. Also fix pre-commit issues from upstream merge: end-of-file newlines, trailing whitespace, and isort import ordering.
|
Fixed the failing tests and pre-commit issues in the latest commit (8be0661):
|
|
Hi @wyf7107 , can you please review this. |
|
@wyf7107 — would you be able to take a look at this small security fix when you have a moment? It adds a path-boundary check to A couple of related small security PRs are open if it's easier to review the class together: #5615 (path boundary for the built-in agent file tools / |
|
@wukath friendly ping — this one's been green and waiting for review for a couple of weeks. It's a small path-traversal fix in |
Merge #5826 ## Summary `resolve_agent_reference` in `config_agent_utils.py` accepted absolute `config_path` values unconditionally and joined relative paths without any boundary validation. An attacker-controlled `config_path` field in an agent YAML could traverse outside the intended agent directory. **Vulnerable pattern (before):** ```python if os.path.isabs(ref_config.config_path): return from_config(ref_config.config_path) # absolute accepted else: return from_config(os.path.join(agent_dir, ref_config.config_path)) # no ".." check ``` **PoC config:** ```yaml tools: - tool_class: AgentTool args: agent: config_path: "../../../../../../etc/passwd" ``` This causes `open("/etc/passwd", "r")` server-side. A `FileNotFoundError` vs `ValidationError` difference also leaks path existence. ## Fix - Reject absolute `config_path` values with `ValueError` - Normalize the joined path and verify it stays within the parent agent directory before calling `from_config` ## Related Same vulnerability exists in `adk-java` (PR: google/adk-java#...) and `adk-go` (PR: google/adk-go#...) — fix pattern is identical. Note: This is distinct from the `resolve_code_reference` RCE (different function, different field, file-read impact vs code execution). Co-authored-by: Xuan Yang <xygoogle@google.com> COPYBARA_INTEGRATE_REVIEW=#5826 from adilburaksen:fix/config-path-traversal 4630cd9 PiperOrigin-RevId: 936810988
|
Thank you @adilburaksen for your contribution! 🎉 Your changes have been successfully imported and merged via Copybara in commit 171ae9e. Closing this PR as the changes are now in the main branch. |
Summary
resolve_agent_referenceinconfig_agent_utils.pyaccepted absoluteconfig_pathvalues unconditionally and joined relative paths without any boundary validation. An attacker-controlledconfig_pathfield in an agent YAML could traverse outside the intended agent directory.Vulnerable pattern (before):
PoC config:
This causes
open("/etc/passwd", "r")server-side. AFileNotFoundErrorvsValidationErrordifference also leaks path existence.Fix
config_pathvalues withValueErrorfrom_configRelated
Same vulnerability exists in
adk-java(PR: google/adk-java#...) andadk-go(PR: google/adk-go#...) — fix pattern is identical.Note: This is distinct from the
resolve_code_referenceRCE (different function, different field, file-read impact vs code execution).