T09 · Insecure Skill Coding Practices
- Location
scripts/followup.sh:11- Finding
Command Execution Through Unsafe Delay Arithmetic Evaluation
- Content
View full analysis
Vulnerability Details
File Location:
scripts/followup.sh, lines 11–22
Vulnerability Type: Shell command injection through arithmetic expansion
Risk Level: HighVulnerable Code:
bash # Parse delay to seconds parse_seconds() { local d="$1" if [[ "$d" =~ ^([0-9]+)s$ ]]; then echo "${BASH_REMATCH[1]}" elif [[ "$d" =~ ^([0-9]+)m$ ]]; then echo $(( BASH_REMATCH[1] * 60 )) elif [[ "$d" =~ ^([0-9]+)h$ ]]; then echo $(( BASH_REMATCH[1] * 3600 )) else echo "$d" fi } SECS=$(parse_seconds "$DELAY") AT=$(date -u -d "@$(( $(date +%s) + SECS ))" +"%Y-%m-%dT%H:%M:%SZ")Technical Analysis
The parser accepts recognized values such as
30s,5m, and2h, but returns unrecognized input unchanged. That attacker-controlled result is subsequently referenced inside Bash arithmetic expansion.Bash arithmetic expressions recursively interpret variable values as arithmetic syntax. In particular, crafted array-subscript expressions may cause embedded command substitutions to be evaluated. Quoting the original script argument does not make its later use in an arithmetic context safe.
The vulnerable behavior can be avoided only by rejecting invalid input rather than returning it for arithmetic evaluation.
Attack Path
- An attacker gains control over, or influences, the first argument passed to
followup.sh. - The attacker supplies a quoted malformed delay containing arithmetic syntax and command substitution, such as:
bash bash scripts/followup.sh 'x[$(touch /tmp/pwn)0]' 'task' - The value does not match any supported delay expression, so
parse_secondsreturns it unchanged. - The value is assigned to
SECS. - Bash evaluates
SECSas part of$(( $(date +%s) + SECS )). - The embedded command substitution executes with the privileges of the process running the Skill.
Impact Assessment
Successful exploitation permits arbitrary local comman ...[truncated 537 chars]
- An attacker gains control over, or influences, the first argument passed to
- Remediation
View remediation
Remediation Suggestions
- Reject every delay that does not exactly match an approved format.
- Convert only the numeric regular-expression capture to seconds.
- Verify that the converted result consists exclusively of decimal digits before using it in arithmetic.
- Apply reasonable upper and lower bounds to prevent immediate or excessively distant schedules.
- Return a nonzero exit status for malformed input instead of echoing it.
- Avoid evaluating any user-controlled string as a Bash arithmetic expression.
Example hardened implementation:
bash parse_seconds() { local d="$1" local number unit if [[ ! "$d" =~ ^([0-9]+)([smh])$ ]]; then printf 'Invalid delay: %s\n' "$d" >&2 return 1 fi number="${BASH_REMATCH[1]}" unit="${BASH_REMATCH[2]}" case "$unit" in s) printf '%d\n' "$number" ;; m) printf '%d\n' "$((10#$number * 60))" ;; h) printf '%d\n' "$((10#$number * 3600))" ;; esac } SECS="$(parse_seconds "$DELAY")" || exit 2 [[ "$SECS" =~ ^[0-9]+$ ]] || exit 2 AT="$(date -u -d "@$(( $(date +%s) + 10#$SECS ))" +"%Y-%m-%dT%H:%M:%SZ")"Add regression tests covering valid delays, malformed units, whitespace, signs, arithmetic operators, array syntax, and command substitutions.