T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/anchor.sh:65
- Finding
- Command Injection Through Unvalidated DAYS_BACK Arithmetic Expression<![CDATA[ ## Vulnerability Details **File Location**: `scripts/anchor.sh`, lines 65–66 and 113–114 **Vulnerability Type**: Shell command injection through recursive Bash arithmetic evaluation **Risk Level**: High ### Vulnerable Code ```bash --days) DAYS_BACK="$2" shift 2 ;; ``` The same value can also be supplied through the environment: ```bash DAYS_BACK="${DAYS_BACK:-2}" ``` It is subsequently evaluated as a Bash arithmetic expression: ```bash get_daily_files() { local files=() for i in $(seq 0 $((DAYS_BACK - 1))); do ``` ### Technical Analysis The script accepts `DAYS_BACK` from either the `DAYS_BACK` environment variable or the `--days` command-line argument without validating that it contains only a bounded positive integer. The value is later used inside: ```bash $((DAYS_BACK - 1)) ``` Bash arithmetic evaluation does not necessarily treat variable contents as inert numeric text. Variable values can be recursively interpreted as arithmetic expressions. Crafted expressions involving array subscripts and command substitution can therefore cause shell commands to execute when the arithmetic expansion is evaluated. In addition, an excessively large numeric value can make `seq` generate an extremely large sequence and cause excessive CPU consumption, memory consumption, or execution time. A zero, negative, malformed, or missing `--days` value can also cause erroneous behavior or abrupt termination under `set -e`. ### Attack Path 1. An attacker gains control over the arguments used to invoke `anchor.sh` or the `DAYS_BACK` environment variable. 2. The attacker supplies a crafted Bash arithmetic expression instead of an integer. 3. The script stores the value without validation. 4. `get_daily_files` inserts the value into `$((DAYS_BACK - 1))`. 5. Bash recursively evaluates the attacker-controlled arithmetic expression. 6. Any embedded command substitution executes with the privileges and environment of the user running the Skill. ...[truncated 700 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate the final `DAYS_BACK` value before any arithmetic expansion: ```bash if [[ ! "$DAYS_BACK" =~ ^[1-9][0-9]*$ ]]; then printf 'Error: --days must be a positive integer.\n' >&2 exit 1 fi if (( DAYS_BACK > 365 )); then printf 'Error: --days must not exceed 365.\n' >&2 exit 1 fi ``` Also verify that `--days` has an argument before reading `$2`: ```bash --days) if [[ $# -lt 2 ]]; then printf 'Error: --days requires a value.\n' >&2 exit 1 fi DAYS_BACK="$2" shift 2 ;; ``` Apply validation after parsing so it covers both the environment variable and command-line input. Keep a reasonable upper bound to prevent resource-exhaustion attacks. For additional hardening: - Use `set -euo pipefail`. - Use `readonly` for validated configuration values where practical. - Avoid feeding untrusted strings into Bash arithmetic contexts. - Add tests for missing, negative, zero, malformed, expression-based, and excessively large values. ]]>