T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:91
- Finding
- User-Controlled Spec Filename Enables Python Code Injection<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 91–103, with repeated unsafe interpolation at lines 151 and 228 **Vulnerability Type**: Python source-code injection through user-controlled filename interpolation **Risk Level**: High ### Vulnerable Code ```bash if [[ "$SPEC_FILE" == *.yaml ]] || [[ "$SPEC_FILE" == *.yml ]]; then python3 -c "import yaml,sys; yaml.safe_load(open('$SPEC_FILE'))" 2>&1 && echo "✅ YAML syntax valid" || echo "❌ YAML syntax error" elif [[ "$SPEC_FILE" == *.json ]]; then python3 -c "import json,sys; json.load(open('$SPEC_FILE'))" 2>&1 && echo "✅ JSON syntax valid" || echo "❌ JSON syntax error" fi python3 -c " import yaml, json, sys try: with open('$SPEC_FILE') as f: spec = yaml.safe_load(f) if '$SPEC_FILE'.endswith(('.yaml','.yml')) else json.load(f) ``` The same unsafe construction is repeated when generating endpoint inventories and test commands: ```bash python3 -c " import yaml, json, sys spec_file = '$SPEC_FILE' with open(spec_file) as f: ``` ```bash python3 -c " import yaml, json spec_file = '$SPEC_FILE' with open(spec_file) as f: ``` ### Technical Analysis The user-provided `SPEC_FILE` value is interpolated directly into Python source passed to `python3 -c`. Shell quoting does not make the resulting Python string safe. A filename containing a single quote can terminate the Python string literal and introduce additional Python statements. Because Python provides direct access to operating-system functionality, successful injection can invoke commands, read files, alter project content, or perform network operations with the privileges of the user running the Skill. This exceeds the minimum privileges needed to parse an OpenAPI document. The file path should be treated strictly as data and passed through an argument or environment variable rather than incorporated into executable source. ### Attack Path 1. An attacker creates or supplies an OpenAPI file with a specially crafted fi ...[truncated 1054 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Pass the path as a positional argument instead of interpolating it into Python source: ```bash python3 -c ' import sys import yaml with open(sys.argv[1], encoding="utf-8") as file: yaml.safe_load(file) ' "$SPEC_FILE" ``` For logic supporting both JSON and YAML: ```bash python3 - "$SPEC_FILE" <<'PY' import json import sys import yaml spec_file = sys.argv[1] with open(spec_file, encoding="utf-8") as file: if spec_file.lower().endswith((".yaml", ".yml")): spec = yaml.safe_load(file) else: spec = json.load(file) PY ``` Apply this argument-based pattern to every embedded Python block, including lines 91, 93, 100, 151, and 228. Additional hardening should include: - Rejecting filenames containing NUL characters or unsupported extensions. - Resolving and validating the path before opening it. - Distinguishing local file input from URL input explicitly. - Avoiding dynamic source construction for all user-controlled values. - Adding regression tests using filenames containing quotes, spaces, newlines, and shell metacharacters. ]]>