T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/tts-local.sh:29
- Finding
- Argument Injection Through Unquoted Speaker Parameter Expansion<![CDATA[ ## Vulnerability Details **File Location**: `scripts/tts-local.sh`, lines 29-53 **Vulnerability Type**: Argument injection caused by unsafe shell word splitting **Risk Level**: Medium ### Vulnerable Code ```bash while [[ "$#" -gt 0 ]]; do case $1 in -o|--output) OUTPUT="$2"; shift ;; -s|--speaker) SPEAKER_PARAM="--tts-speaker-file $2"; shift ;; -t|--temp) TEMP="$2"; shift ;; -h|--help) usage ;; *) TEXT="$1" ;; esac shift done if [ -z "$TEXT" ]; then usage fi # Run llama-tts llama-tts \ -m "$MODEL" \ -mv "$VOCODER" \ -p "$TEXT" \ -o "$OUTPUT" \ --temp "$TEMP" \ --repeat-penalty "$REP_PENALTY" \ --repeat-last-n "$REP_LAST_N" \ --top-k "$TOP_K" \ --top-p "$TOP_P" \ --min-p "$MIN_P" \ $SPEAKER_PARAM ``` ### Technical Analysis The speaker option and its user-controlled value are concatenated into the scalar variable `SPEAKER_PARAM`. The variable is subsequently expanded without quotation. Bash applies word splitting and pathname expansion to an unquoted variable expansion. Consequently, whitespace in the supplied speaker value produces additional command-line arguments rather than remaining part of a single filename. An attacker who can control the speaker argument can inject additional options accepted by `llama-tts`. For example, a caller could pass a single shell-quoted value containing extra arguments: ```bash scripts/tts-local.sh --speaker 'reference.wav -o /tmp/alternate.wav' 'text' ``` The wrapper can expand that value into arguments equivalent to: ```text --tts-speaker-file reference.wav -o /tmp/alternate.wav ``` The exact result depends on how `llama-tts` handles duplicate options and option ordering. This flaw does not directly cause arbitrary shell command execution because shell metacharacters introduced through variable expansion are not reparsed as shell syntax. It is nevertheless a command-line argument injection vulnerability. Th ...[truncated 1623 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Represent optional arguments as a Bash array so each value retains its argument boundary: ```bash SPEAKER_ARGS=() while [[ "$#" -gt 0 ]]; do case "$1" in -o|--output) [[ $# -ge 2 ]] || { echo "Error: $1 requires a value." >&2 usage } OUTPUT="$2" shift 2 ;; -s|--speaker) [[ $# -ge 2 ]] || { echo "Error: $1 requires a value." >&2 usage } SPEAKER_ARGS=(--tts-speaker-file "$2") shift 2 ;; -t|--temp) [[ $# -ge 2 ]] || { echo "Error: $1 requires a value." >&2 usage } TEMP="$2" shift 2 ;; -h|--help) usage ;; --) shift TEXT="$*" break ;; -*) echo "Error: unknown option: $1" >&2 usage ;; *) TEXT="$1" shift ;; esac done llama-tts \ -m "$MODEL" \ -mv "$VOCODER" \ -p "$TEXT" \ -o "$OUTPUT" \ --temp "$TEMP" \ --repeat-penalty "$REP_PENALTY" \ --repeat-last-n "$REP_LAST_N" \ --top-k "$TOP_K" \ --top-p "$TOP_P" \ --min-p "$MIN_P" \ "${SPEAKER_ARGS[@]}" ``` Additionally: - Validate that the speaker file exists, is a regular file, and is readable. - Validate output paths according to the intended trust boundary. - Validate temperature as a numeric value within the supported range. - Reject unknown options rather than treating them as text. - Use `set -euo pipefail` where compatible to make parsing and execution failures explicit. ]]>