T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/video-summarize.sh:466
- Finding
- Arbitrary Python Code Execution Through Output Directory Interpolation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/video-summarize.sh`, lines 466-470 **Vulnerability Type**: User-controlled path interpolated into dynamically evaluated Python source **Risk Level**: High ### Vulnerable Code ```bash TITLE=$($PYTHON -c "import json; print(json.load(open('$OUTPUT_DIR/metadata.json')).get('title', 'Unknown'))" 2>/dev/null || echo "Unknown") UPLOADER=$($PYTHON -c "import json; print(json.load(open('$OUTPUT_DIR/metadata.json')).get('uploader', 'Unknown'))" 2>/dev/null || echo "Unknown") DURATION=$($PYTHON -c "import json; print(json.load(open('$OUTPUT_DIR/metadata.json')).get('duration_string', 'Unknown'))" 2>/dev/null || echo "Unknown") DURATION_SEC=$($PYTHON -c "import json; print(int(json.load(open('$OUTPUT_DIR/metadata.json')).get('duration', 0)))" 2>/dev/null || echo "0") THUMBNAIL=$($PYTHON -c "import json; print(json.load(open('$OUTPUT_DIR/metadata.json')).get('thumbnail', ''))" 2>/dev/null || echo "") ``` The same unsafe interpolation pattern also appears at lines 917, 1006, and 1075. ### Technical Analysis The second positional command-line argument is accepted as `OUTPUT_DIR`. The `validate_output_dir()` function rejects `..` and a limited set of sensitive system directories, but does not reject quote characters or Python syntax. `OUTPUT_DIR` is subsequently inserted directly into source code supplied to `python -c`. Shell quoting does not make this safe because the shell first expands the variable into the double-quoted command argument, after which Python interprets the resulting string as executable source code. An attacker can include a single quote and Python expression syntax in the output directory. For example, a path shaped like the following can cause a function call to be evaluated while Python constructs the argument to `open()`: ```text /tmp/'+str(__import__('os').system('id'))+'x ``` This changes the effective Python expression and invokes `os.system()` before normal file handling finis ...[truncated 1370 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Do not construct executable Python source using interpolated paths. Pass all values through environment variables or command-line arguments. For example: ```bash OUTPUT_DIR="$OUTPUT_DIR" "$PYTHON" -c ' import json import os metadata_path = os.path.join(os.environ["OUTPUT_DIR"], "metadata.json") with open(metadata_path, encoding="utf-8") as metadata_file: print(json.load(metadata_file).get("title", "Unknown")) ' ``` A preferable long-term fix is to place metadata extraction in a dedicated Python script and pass the metadata path as a normal argument: ```bash "$PYTHON" "$SCRIPT_DIR/read-metadata-field.py" \ "$OUTPUT_DIR/metadata.json" title ``` Additional hardening should include: 1. Replace every path interpolated into `python -c`, including the patterns at lines 917, 1006, and 1075. 2. Reject control characters, newlines, and NUL bytes in user-supplied paths. 3. Canonicalize output paths before applying restricted-directory policies. 4. Add automated tests using paths containing single quotes, double quotes, spaces, newlines, shell metacharacters, and Python expression syntax. 5. Avoid relying on character blacklists as the primary injection defense; values should remain data rather than executable source. ]]>