T09 · Insecure Skill Coding Practices
Error
- Location
cleanup.sh:5- Finding
Arbitrary Command Execution Through Unvalidated Arithmetic Input
- Content
View full analysis
[keep-count]}" PIECE_ID="${2:?Provide piece ID}" KEEP="${3:-3}" # Keep last N versions by default VERSION_DIR="$WORKSPACE/versions/$PIECE_ID" [[ -d "$VERSION_DIR" ]] || { echo "❌ No versions found for: $PIECE_ID"; exit 1; } TOTAL=$(ls -1 "$VERSION_DIR" | wc -l | tr -d ' ') DELETE=$((TOTAL - KEEP)) ``` ### Technical Analysis The third command-line argument is accepted as `KEEP` without confirming that it is a decimal integer. Bash arithmetic expressions recursively evaluate variable values as arithmetic syntax. Certain crafted expressions, particularly expressions involving array subscripts and command substitutions, can therefore cause shell commands to run during evaluation. Quoting the original assignment does not make the later arithmetic evaluation safe. The dangerous operation occurs when `KEEP` is interpreted inside: ```bash DELETE=$((TOTAL - KEEP)) ``` This evaluation occurs before the script asks the user to confirm deletion. Consequently, the deletion confirmation does not protect against command execution. ### Attack Path 1. An attacker influences the third argument passed to `cleanup.sh`. 2. The attacker supplies a value containing malicious Bash arithmetic syntax. 3. The script stores the value in `KEEP` without validation. 4. Bash recursively evaluates that value while calculating `DELETE`. 5. Embedded command substitution executes with the privileges of the user or Agent running the Skill. 6. Execution occurs before the confirmation prompt is displayed. ### Impact Assessment Successful exploitation permits arbitrary local command execution under the invoking account. The attacker could read or modify any file accessible to that account, alter writing projects, execute installed programs, or use the ...[truncated 233 chars]- Remediation
View remediation
&2 exit 1 fi KEEP=$((10#$KEEP)) if (( KEEP > 10000 )); then echo "❌ Keep count exceeds the permitted limit." >&2 exit 1 fi DELETE=$((TOTAL - KEEP)) ``` Additional hardening should include: - Reject negative numbers, expressions, whitespace, signs, and non-decimal notation. - Apply a reasonable upper bound. - Keep confirmation logic, but do not treat it as an input-validation control. - Add regression tests using arithmetic operators, array syntax, command substitutions, negative values, and very large values. ]]>