T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/publish.sh:77
- Finding
- Arbitrary Python Code Execution Through Unsafe Source Interpolation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/publish.sh:77, 88-92, 97-102` **Vulnerability Type**: Python code injection **Risk Level**: High ### Vulnerable Code ```bash CURRENT_VERSION=$(python3 -c "import json; print(json.load(open('$META_FILE')).get('version', '1.0.0'))") ``` ```bash VERSION=$(python3 -c " parts = '$CURRENT_VERSION'.split('.') parts[-1] = str(int(parts[-1]) + 1) print('.'.join(parts)) ") ``` ```bash python3 -c " import json meta = {'slug': '$SKILL_NAME', 'version': '$VERSION'} with open('$META_FILE', 'w') as f: json.dump(meta, f, indent=2) " ``` ### Technical Analysis The script constructs Python source code dynamically and passes it to `python3 -c`. Several values are placed directly inside single-quoted Python string literals without escaping: - `META_FILE` is derived from the user-selected skill directory. - `SKILL_NAME` is derived from the basename of that directory. - `CURRENT_VERSION` is read from the skill's attacker-controllable `_meta.json`. - `VERSION` can be supplied directly through the `--version` command-line argument. Shell quoting does not make these values safe within the subsequently generated Python source. A value containing a single quote can terminate its intended Python string literal. Additional syntactically valid Python statements can then be introduced into the code executed by `python3 -c`. This issue is distinct from shell command injection: the shell invokes the intended Python executable, but Python parses attacker-controlled content as executable source rather than data. ### Attack Path 1. An attacker prepares or controls a skill directory submitted to the publishing workflow. 2. The attacker places a crafted value in one of the following sources: - The skill directory path or basename. - The `version` property in `_meta.json`. - The `--version` command-line argument. 3. The crafted value contains a quote that closes the generated Python string, followed by valid Pyt ...[truncated 1545 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Never interpolate paths, metadata, or command-line arguments into Python source code. Pass all values as ordinary process arguments and read them through `sys.argv`. For example, replace metadata reading with: ```bash CURRENT_VERSION=$( python3 - "$META_FILE" <<'PY' import json import sys with open(sys.argv[1], encoding="utf-8") as handle: metadata = json.load(handle) print(metadata.get("version", "1.0.0")) PY ) ``` Replace version incrementing with argument-based processing and strict validation: ```bash VERSION=$( python3 - "$CURRENT_VERSION" <<'PY' import re import sys version = sys.argv[1] if not re.fullmatch(r"(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)", version): raise SystemExit("Invalid semantic version") major, minor, patch = map(int, version.split(".")) print(f"{major}.{minor}.{patch + 1}") PY ) ``` Write `_meta.json` by passing every value separately: ```bash python3 - "$META_FILE" "$SKILL_NAME" "$VERSION" <<'PY' import json import sys meta_file, skill_name, version = sys.argv[1:] metadata = { "slug": skill_name, "version": version, } with open(meta_file, "w", encoding="utf-8") as handle: json.dump(metadata, handle, indent=2) PY ``` Additional hardening should include: 1. Validate both existing and user-supplied versions against a strict semantic-version expression before any file modification. 2. Validate `SKILL_NAME` against the same slug constraints used for published metadata, such as `^[a-z0-9-]+$`. 3. Reject missing values for `--version` and `--changelog` before shifting command-line arguments. 4. Resolve and verify the skill directory before use, and ensure it is within an approved publishing root if the workflow processes untrusted paths. 5. Run the publishing process with a dedicated, least-privileged account and narrowly scoped ClawHub credentials. 6. Add regression tests containing quotes, newlines, backslashes, and Python syntax in directory names, metadata ...[truncated 40 chars]