T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/git-sync.py:1321
- Finding
- Shell Command Injection Through ClawHub Publication Metadata<![CDATA[ ## Vulnerability Details **File Location**: `scripts/git-sync.py:1321-1326` **Vulnerability Type**: OS command injection through untrusted metadata and `shell=True` **Risk Level**: Critical ### Vulnerable Code ```python def step_clawhub_publish(name: str, version: str): # v2.37.0 多仓库：skill 仓库根下直接是技能目录 sd = get_work_repo("skill") / name if not sd.is_dir(): print(" ❌ 技能目录不存在"); return meta = json.loads((sd/"_meta.json").read_text(encoding="utf-8")) slug = meta.get("slug",name) cmd = f'npx clawhub publish "{sd}" --slug "{slug}" --name "{meta.get("displayName",name)}" --version "{version}" --changelog "v{version}"' if meta.get("tags"): cmd += ' --tags "' + ",".join(meta["tags"]) + '"' r = subprocess.run(cmd, capture_output=True, text=True, shell=True) ``` ### Technical Analysis The command is assembled as one shell command string and then executed with `shell=True`. Several interpolated values originate from the project’s `_meta.json` file: - `slug` - `displayName` - `tags` Wrapping those values in double quotes does not make them safe. An attacker can include a quotation mark followed by shell metacharacters to terminate the intended argument and append another command. The exact metacharacters vary by operating system, but the flaw applies to both POSIX shells and Windows command processors. The project already contains a safer implementation in `scripts/clawhub_publish.py`, where arguments are passed as a list. The vulnerable integrated implementation does not use that protection. ### Attack Path 1. An attacker supplies or modifies a project that the user intends to publish. 2. The attacker inserts shell syntax into an `_meta.json` field, for example a `displayName` that closes the quoted argument and appends a command. 3. The user invokes the Skill with ClawHub publishing enabled. 4. `step_clawhub_publish()` reads the attacker-controlled metadata. 5. The function interpolates the value into `cmd`. 6. `s ...[truncated 966 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove `shell=True` and pass every argument as a separate list element: ```python cmd = [ "npx", "--no-install", "clawhub", "publish", str(sd), "--slug", slug, "--name", meta.get("displayName", name), "--version", version, "--changelog", f"v{version}", ] if meta.get("tags"): cmd.extend(["--tags", ",".join(meta["tags"])]) result = subprocess.run( cmd, capture_output=True, text=True, shell=False, check=False, ) ``` 2. Reuse the argument-list implementation in `scripts/clawhub_publish.py` rather than maintaining a second command-building path. 3. Validate `slug`, `displayName`, tags, project name, and version before command execution: - Impose reasonable maximum lengths. - Require `slug` and project names to match a strict allowlist such as `[A-Za-z0-9._-]+`. - Require tags to be strings and reject control characters. - Validate versions using the expected version grammar. 4. Do not attempt to fix this solely through manual shell escaping. Avoiding shell interpretation is the more reliable control. 5. Add regression tests containing quotation marks, command separators, newlines, substitutions, and platform-specific shell metacharacters. ]]>