T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/indeed_poll_and_fetch.sh:52
- Finding
- Command Injection Through Unvalidated Arithmetic Input<![CDATA[ ## Vulnerability Details **File Location**: `scripts/indeed_poll_and_fetch.sh:52-58, 104` **Vulnerability Type**: Bash arithmetic-expression injection **Risk Level**: High ### Vulnerable Code ```bash while [[ $# -gt 0 ]]; do case "$1" in --help) show_help ;; --timeout|--interval|--description|--dataset-type) [[ -n "${2:-}" ]] || { echo "Error: $1 requires a value" >&2; exit 1; } case "$1" in --timeout) TIMEOUT="$2" ;; --interval) INTERVAL="$2" ;; --description) DESCRIPTION="$2" ;; --dataset-type) DATASET_TYPE="$2" ;; esac shift 2 ;; ``` The unvalidated value is subsequently used in an arithmetic comparison: ```bash while [[ "$elapsed" -lt "$TIMEOUT" ]]; do ``` ### Technical Analysis The `--timeout` argument is assigned directly to `TIMEOUT` without verifying that it is a decimal integer. It is subsequently used as an operand in a Bash arithmetic comparison. Bash arithmetic contexts parse operand values as arithmetic expressions rather than inert strings. Arithmetic expressions can recursively resolve variable names and array subscripts, and specially constructed expressions can cause additional shell expansions. Consequently, treating untrusted input as an arithmetic operand may permit command execution rather than merely causing a numeric parsing error. The same validation omission affects `--interval`. Although that value is first passed as a quoted argument to `sleep`, it is also later evaluated in: ```bash elapsed=$((elapsed + INTERVAL)) ``` Strict numeric validation is therefore required for both options. ### Attack Path 1. An attacker supplies or induces the agent to supply a crafted nonnumeric value to `--timeout`. 2. `parse_args` stores the value verbatim in `TIMEOUT`. 3. Execution reaches the loop condition at line 104. 4. Bash interprets the supplied value as part of an arithmetic expression. 5. Malicious expansion embedded in that expression can execute a local co ...[truncated 771 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate both arguments as bounded decimal integers before assigning them: ```bash validate_positive_integer() { local name="$1" local value="$2" local maximum="$3" if [[ ! "$value" =~ ^[0-9]+$ ]] || (( 10#$value < 1 || 10#$value > maximum )); then echo "Error: ${name} must be an integer between 1 and ${maximum}" >&2 exit 1 fi } case "$1" in --timeout) validate_positive_integer "--timeout" "$2" 3600 TIMEOUT="$2" ;; --interval) validate_positive_integer "--interval" "$2" 300 INTERVAL="$2" ;; esac ``` Additional hardening should include: - Use `10#` when converting validated decimal input to prevent unintended octal interpretation. - Enforce sensible upper bounds to prevent excessive polling or denial of service. - Reject zero and negative values. - Add regression tests containing whitespace, signs, arithmetic operators, variable names, array syntax, and command-substitution syntax. - Avoid placing any untrusted string into Bash arithmetic contexts unless it has first passed a strict decimal-only allowlist. ]]>