T09 · Insecure Skill Coding Practices
Error
- Location
scripts/logcat_capture.sh:51- Finding
Arbitrary Shell Command Injection Through eval-Based Command Construction
- Content
View full analysis
/dev/null || echo "") if [ -z "$PID" ]; then echo "Process not running: $PKG"; exit 1; fi CMD="$CMD --pid=$PID" ;; tag) CMD="$CMD -s $TAG" ;; esac if [ -n "$OUTPUT" ]; then eval "$CMD" > "$OUTPUT" LINES=$(wc -l < "$OUTPUT" | tr -d ' ') echo "Captured $LINES lines -> $OUTPUT" else eval "$CMD" fi ``` ### Technical Analysis The script constructs a shell command as a string and executes it through `eval`. The `SERIAL` and `TAG` parameters originate from positional command-line arguments and are inserted into that command string without shell escaping or restrictive validation. Quoting a variable while passing it to `eval` does not make its contents safe. `eval` reparses the expanded string as shell syntax. Consequently, shell metacharacters, command substitutions, redirections, and command separators embedded in `TAG` or `SERIAL` are interpreted by the host shell. The PID lookup also expands the unquoted string variable `$SERIAL_FLAG`, although the primary arbitrary-code-execution sink is the subsequent use of `eval`. ### Attack Path 1. An attacker causes the Skill to invoke the logcat helper with a crafted tag or serial. 2. For example, a tag containing a command substitution can be passed as a literal argument: ```bash bash scripts/logcat_capture.sh tag '$(touch /tmp/adb-injected)' ``` 3. The script appends the value to `CMD`: ```bash CMD="$CMD -s $TAG" ``` 4. `eval "$CMD"` reparses the command substitution as active shell syntax. 5. The injected `touch` command executes on the host, independently of whet ...[truncated 935 chars]- Remediation
View remediation
/dev/null || true) if [ -z "$PID" ]; then echo "Process not running: $PKG" exit 1 fi cmd+=("--pid=$PID") ;; tag) cmd+=(-s "$TAG") ;; esac if [ -n "$OUTPUT" ]; then "${cmd[@]}" > "$OUTPUT" else "${cmd[@]}" fi ``` Additional hardening should include: - Validate device serials against an explicit expected format. - Validate package names using an Android package-name allowlist pattern. - Reject control characters in tags and output paths. - Use `--` where supported to terminate option parsing. - Add regression tests using tags and serials containing spaces, quotes, semicolons, dollar signs, and command substitutions. ]]>