T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/subagent_orchestrator.py:247
- Finding
- Arbitrary Python Code Execution Through Untrusted Workflow Configuration<![CDATA[ ## Vulnerability Details **File Location**: `scripts/subagent_orchestrator.py`, lines 117-122 and 247-260 **Vulnerability Type**: Configuration-driven dynamic import and unsafe expression evaluation **Risk Level**: High ### Vulnerable Code ```python module_name = agent_config["module"] class_name = agent_config["class"] # Dynamic module import module = importlib.import_module(module_name.replace('/', '.')) agent_class = getattr(module, class_name) ``` ```python def _evaluate_condition(self, condition: str) -> bool: """Evaluate a condition expression.""" try: if condition == "user_confirmed == true": return self.context.get("user_confirmed", False) is True elif condition == "is_valid == true": return self.context.get("is_valid", False) is True else: local_vars = {**self.context} for task_id, result in self.results.items(): local_vars.update(result.outputs) return eval(condition, {}, local_vars) except Exception as e: logger.warning(f"Failed to evaluate condition '{condition}': {e}") return False ``` ### Technical Analysis `SubagentOrchestrator` accepts a configurable workflow file and uses values from that file to control two executable Python mechanisms: 1. Agent module and class names are passed to `importlib.import_module()` and `getattr()`. 2. Workflow condition strings are passed directly to Python's `eval()`. Passing an empty dictionary as the globals argument to `eval()` does not reliably create a secure sandbox. Python can populate built-ins, allowing a malicious expression to access functions such as `__import__` and then invoke operating-system or file APIs. Context objects and task outputs also become available as local variables, increasing the available attack surface. The dynamic import path independently permits execution of module-level code when a configuration identifies an attacker-controlled impo ...[truncated 1563 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove `eval()` entirely and implement a strict condition parser. - Permit only predefined predicates such as `user_confirmed` and `is_valid`. - Permit only explicitly supported equality or Boolean operations. - Reject unknown identifiers, function calls, attribute access, indexing, and imports. 2. Replace configuration-controlled imports with an in-code allowlist: ```python AGENT_CLASSES = { "lore_bible_manager": LoreBibleManager, "conflict_detector": ConflictDetector, "profile_generator": CharacterProfileGenerator, } ``` 3. Do not accept arbitrary module or class names from workflow JSON. 4. Validate workflow files against a restrictive JSON schema before loading them. 5. Require workflow configuration files to be stored in a trusted directory with appropriate ownership and permissions. 6. If custom workflows are required, parse expressions with `ast.parse()` and allow only a minimal set of safe AST nodes; do not compile or evaluate arbitrary syntax. 7. Add regression tests using malicious conditions and module names to verify that configuration cannot invoke imports, functions, attributes, or system commands. ]]>