T09 · Insecure Skill Coding Practices
Error
- Location
- click.sh:115
- Finding
- Command Injection Through Unvalidated Arithmetic Expressions<![CDATA[ ## Vulnerability Details **File Location**: `click.sh:16-22, 110-116` **Vulnerability Type**: Command injection through Bash arithmetic expansion **Risk Level**: High ### Vulnerable Code ```bash --x) X="$2" shift 2 ;; --y) Y="$2" shift 2 ;; ``` ```bash # Get window position GEOM=$(xdotool getwindowgeometry --shell "$WIN_ID") WIN_X=$(echo "$GEOM" | grep "^X=" | cut -d= -f2) WIN_Y=$(echo "$GEOM" | grep "^Y=" | cut -d= -f2) # Calculate absolute coordinates X=$((WIN_X + X)) Y=$((WIN_Y + Y)) ``` ### Technical Analysis The `--x` and `--y` arguments are accepted as arbitrary strings and are not validated as decimal integers. When window-relative clicking is enabled, these values are evaluated inside Bash arithmetic expansions. Bash arithmetic evaluation parses operands as arithmetic expressions rather than inert numeric strings. Values referenced through arithmetic variables may be evaluated recursively. Crafted expressions can therefore trigger additional shell expansion, including command substitution in constructs such as array subscripts. This flaw is reachable only through the window-relative branch, because the vulnerable arithmetic expressions are evaluated when `--window` is supplied. Quoting the original assignment does not make subsequent arithmetic evaluation safe. ### Attack Path 1. An attacker causes the script to be invoked with a valid window name and a malicious `--x` or `--y` expression. 2. The script stores the expression without numeric validation. 3. `xdotool` locates the requested X11 window and returns its geometry. 4. The script evaluates the attacker-controlled value in: ```bash X=$((WIN_X + X)) ``` or: ```bash Y=$((WIN_Y + Y)) ``` 5. A crafted arithmetic expression containing command substitution is evaluated by Bash. 6. The injected command executes with the identity and environment of the user running the skil ...[truncated 964 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate all coordinate values as integers before using them in arithmetic expansion: ```bash if ! [[ "$X" =~ ^-?[0-9]+$ ]] || ! [[ "$Y" =~ ^-?[0-9]+$ ]]; then echo "ERROR: --x and --y must be decimal integers" >&2 exit 1 fi ``` Perform validation before both direct `xdotool` use and window-relative arithmetic. Validate values obtained from `xdotool` as well: ```bash if ! [[ "$WIN_X" =~ ^-?[0-9]+$ ]] || ! [[ "$WIN_Y" =~ ^-?[0-9]+$ ]]; then echo "ERROR: invalid window geometry" >&2 exit 1 fi ``` Additional hardening should include: 1. Enforce sensible upper and lower coordinate bounds. 2. Apply strict integer validation to `--amount`, `--delay`, `--width`, `--height`, and other numeric parameters in the remaining scripts. 3. Avoid evaluating untrusted strings as arithmetic expressions. 4. Add regression tests using arithmetic metacharacters, command substitutions, array syntax, whitespace, signs, and oversized values. ]]>