block yaml deserialization in agent-config code references - #6646
Open
prasanna8585 wants to merge 1 commit into
Open
block yaml deserialization in agent-config code references#6646prasanna8585 wants to merge 1 commit into
prasanna8585 wants to merge 1 commit into
Conversation
_validate_module_reference()'s denylist blocks the standard library plus a short explicit list of stdlib-adjacent modules, but, as its own commit message (a16f6da) states, cannot cover third-party packages in general. adk-python depends on PyYAML unconditionally (pyyaml>=6.0.2,<7, not an optional integration). yaml.unsafe_load / yaml.load / yaml.full_load are single-argument functions that construct arbitrary Python objects from the YAML document they are given, via tags such as !!python/object/apply:os.system. None of these were blocked. Referenced as a tool with no declared args -- the same entry point the already-fixed cProfile.run exploit used -- the resolved function is exposed directly as a callable tool. A functionCall dispatching to it with a malicious YAML string as the argument achieves full RCE. Confirmed dynamically against the real, installed package: the attack succeeds before this change and is blocked after it, with no regression to the existing stdlib denylist or to legitimate third-party references (pydantic.BaseModel, etc.). Fix: add yaml to _BLOCKED_MODULES, matching the file's existing pattern for other named, load-bearing entries not covered by the stdlib check. Adds 6 regression tests. Full test_agent_config.py (121 tests) and the broader tests/unittests/agents/ suite (576 tests) pass with zero regressions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_validate_module_reference()'s denylist blocks the standard library plus a short explicit list of stdlib-adjacent modules -- but, as commit a16f6da's own message states, "cannot cover third-party packages" in general.adk-pythondepends on PyYAML unconditionally (pyyaml>=6.0.2,<7inpyproject.toml, not an optional integration).yaml.unsafe_load/yaml.load/yaml.full_loadare single-argument functions that construct arbitrary Python objects from the YAML document they're given, via tags such as!!python/object/apply:os.system. None of these were blocked.Referenced as a tool with no declared
args-- the same entry point the already-fixedcProfile.runexploit used (see this file's owntest_resolve_tools_blocks_exec_capable_stdlib, whose docstring describes "the path the reported exploit takes") -- the resolved function is exposed directly as a callable tool. A functionCall dispatching to it with a malicious YAML string as the argument achieves full RCE.Verification
Confirmed dynamically against the real, installed package before this fix:
And after this fix, the same input is rejected:
Fix
Add
yamlto_BLOCKED_MODULES, matching this file's existing pattern for other named, load-bearing entries (distutils,test, etc.) that aren't covered by the stdlib-name check.Testing
yaml.unsafe_load/yaml.load/yaml.full_load, both as a direct code reference and as a resolved tool.test_agent_config.py: 121 passed.tests/unittests/agents/suite (excluding 3 files requiring optional extras not installed in my environment --a2a,mcp): 576 passed, 1 skipped, 2 xfailed, zero regressions.test_third_party_module_reference_is_not_blockedstill passes:pydantic.BaseModelremains resolvable).Scope note
This closes the specific, demonstrated gap (a hard, always-installed dependency with an unconditional deserialization gadget). It does not close the general third-party-package gap the original fix's commit message already identifies -- that would need either an allowlist of approved third-party callables or a different trust model for third-party code references, which felt like a larger design discussion better suited to a maintainer decision than a PR.