T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/clawhub_rate_limited_uploader.py:56
- Finding
- Arbitrary Command Execution Through Unsafe Queue Command Processing<![CDATA[ ## Vulnerability Details **File Location**: `scripts/clawhub_rate_limited_uploader.py`, lines 56–110 **Vulnerability Type**: OS command injection caused by attacker-controlled command templates and `shell=True` **Risk Level**: High ### Vulnerable Code ```python def normalize_items(queue_data: Dict[str, Any]) -> List[QueueItem]: items = queue_data.get("items") if not isinstance(items, list): raise SystemExit('Queue JSON must contain an "items" array.') normalized: List[QueueItem] = [] for index, item in enumerate(items): if not isinstance(item, dict): raise SystemExit(f"Queue item #{index} must be an object.") path = item.get("path") if not isinstance(path, str) or not path.strip(): raise SystemExit(f'Queue item #{index} missing non-empty "path".') command = item.get("command", DEFAULT_COMMAND) if not isinstance(command, str) or "{path}" not in command: raise SystemExit(f'Queue item #{index} has invalid "command"; it must be a string containing "{{path}}".') normalized.append(QueueItem(path=path, command=command)) return normalized def ensure_skill_dir(path_str: str) -> Path: path = Path(path_str).expanduser().resolve() if not path.exists(): raise SystemExit(f"Skill path does not exist: {path}") if not path.is_dir(): raise SystemExit(f"Skill path is not a directory: {path}") if not (path / "SKILL.md").exists(): raise SystemExit(f"Skill directory does not contain SKILL.md: {path}") return path def run_publish(item: QueueItem, execute: bool) -> subprocess.CompletedProcess[str] | None: skill_path = ensure_skill_dir(item.path) command_str = item.command.format(path=str(skill_path)) print(f"[info] command: {command_str}") if not execute: return None return subprocess.run( command_str, shell=True, text=True, capture_output=True, chec ...[truncated 3196 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Remove shell command-string execution and invoke the ClawHub executable with a fixed argument vector: ```python def run_publish(item: QueueItem, execute: bool) -> subprocess.CompletedProcess[str] | None: skill_path = ensure_skill_dir(item.path) command = ["clawhub", "publish", str(skill_path)] print(f"[info] command: {shlex.join(command)}") if not execute: return None return subprocess.run( command, shell=False, text=True, capture_output=True, check=False, cwd=str(skill_path.parent), ) ``` Additional hardening should include: 1. Remove the `command` property from the queue schema if the only supported operation is publishing. 2. If customization is essential, represent commands as JSON argument arrays rather than shell strings. 3. Strictly allowlist the executable and operation, such as exactly `clawhub` followed by `publish`. 4. Never pass queue-controlled input to `shell=True`. 5. Treat displayed commands as logs only; use `shlex.join()` for readable output rather than for execution. 6. Reject unexpected queue fields or document and validate a strict schema. 7. Add tests using paths and command values containing quotes, semicolons, substitutions, spaces, and redirection characters to confirm they remain single literal arguments. 8. Restrict queue-file ownership and permissions as defense in depth, while not relying on permissions as a replacement for safe process invocation. ]]>