T09 · Insecure Skill Coding Practices
Error
- Location
scripts/clawhub_rate_limited_uploader.py:56- Finding
Arbitrary Command Execution Through Unsafe Queue Command Processing
- Content
View full analysis
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
View remediation
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. ]]>