T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/convert_audio.sh:53
- Finding
- Command and Argument Injection Through Unvalidated Audio Quality Input<![CDATA[ ## Vulnerability Details **File Location**: `scripts/convert_audio.sh`, lines 17, 31–32, 47–48, 55–56, 63–64, and 76 **Vulnerability Type**: Shell command injection and ffmpeg argument injection **Risk Level**: High ### Vulnerable Code ```bash QUALITY="${3:-}" ``` ```bash if [[ -n "$QUALITY" ]]; then QUAL_OPTS="-b:a ${QUALITY}k" fi ``` ```bash if [[ -n "$QUALITY" ]]; then QUAL_OPTS="-qscale:a $((QUALITY / 40))" # Approximate mapping fi ``` ```bash ffmpeg -y -v warning -i "$INPUT" -codec:a $CODEC $QUAL_OPTS "$OUTPUT" ``` ### Technical Analysis The third positional argument is accepted as `QUALITY` without validating that it is a decimal integer within a permitted range. For OGG output, the value is inserted into a Bash arithmetic expansion: ```bash $((QUALITY / 40)) ``` Bash recursively evaluates variable contents in arithmetic contexts. A crafted value containing an array subscript and command substitution can therefore cause a local command to execute while Bash evaluates the expression. Quoting the argument when initially assigning it to `QUALITY` does not prevent this later arithmetic evaluation. For all output formats that use `QUAL_OPTS`, the assembled options are expanded without quotes: ```bash $QUAL_OPTS ``` This triggers shell word splitting. An attacker can place whitespace and ffmpeg option tokens in the quality argument, causing those tokens to be interpreted as additional command-line arguments rather than as one validated bitrate or quality value. Although `CODEC` is also expanded without quotes, its value is selected from fixed constants and is not directly attacker-controlled. The exploitable input is `QUALITY`. ### Attack Path 1. An attacker supplies or persuades the agent to use a crafted third argument when invoking `convert_audio.sh`. 2. The script stores that argument in `QUALITY` without checking its syntax or range. 3. If the selected output extension is `.ogg`, line 56 evaluates the attacker-controlled ...[truncated 1491 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Strictly validate the quality argument before any arithmetic use.** ```bash if [[ -n "$QUALITY" && ! "$QUALITY" =~ ^[0-9]+$ ]]; then echo "Error: Quality must be a decimal integer." >&2 exit 1 fi ``` 2. **Apply format-specific numeric ranges.** For example, reject zero, negative, excessively large, or otherwise unsupported bitrate values. For OGG, calculate the mapped quality only after validation and clamp or reject results outside ffmpeg's accepted range. ```bash if (( QUALITY < 1 || QUALITY > 512 )); then echo "Error: Quality is outside the supported range." >&2 exit 1 fi ``` 3. **Use a Bash array for ffmpeg options instead of constructing a string.** ```bash qual_opts=() case "$OUT_EXT" in mp3) CODEC="libmp3lame" if [[ -n "$QUALITY" ]]; then qual_opts=(-b:a "${QUALITY}k") else qual_opts=(-qscale:a 2) fi ;; ogg) CODEC="libvorbis" if [[ -n "$QUALITY" ]]; then ogg_quality=$((10#$QUALITY / 40)) qual_opts=(-qscale:a "$ogg_quality") else qual_opts=(-qscale:a 5) fi ;; esac ffmpeg -y -v warning -i "$INPUT" -codec:a "$CODEC" \ "${qual_opts[@]}" "$OUTPUT" ``` 4. **Use `10#` for validated decimal arithmetic** to prevent values with leading zeroes from being interpreted under an unintended numeric base. 5. **Reject unexpected argument counts** so trailing or malformed inputs cannot be silently ignored. 6. **Consider restricting ffmpeg protocols** if the Skill only needs local file conversion. A suitable protocol allowlist reduces the impact of future ffmpeg argument-handling defects. 7. **Add regression tests** covering nonnumeric values, whitespace, shell metacharacters, arithmetic expressions, command substitutions, negative numbers, leading zeroes, and out-of-range values. The tests should verify that validation fails before ffmpeg is invoked. ]]>