T09 · Insecure Skill Coding Practices
Warning
- Location
install.sh:19- Finding
Persistent Shell Command Injection Through Unsanitized Installation Path
- Content
View full analysis
/dev/null; then echo "" >> "$SHELL_RC" echo "# preflight workflow package" >> "$SHELL_RC" echo "export PATH=\"\$PATH:$INSTALL_DIR\"" >> "$SHELL_RC" echo " Added to PATH ($SHELL_RC)" echo " Run: source $SHELL_RC" else echo " PATH already contains preflight; skipping" fi ``` ### Technical Analysis The `--path` argument is accepted without validation and stored directly in `INSTALL_DIR`. Although filesystem operations quote this variable correctly, the installer subsequently interpolates it into executable shell syntax appended to the user's `.bashrc` or `.zshrc`. Shell quoting at the time `echo` runs does not make the generated startup-file content safe. An installation path containing a double quote, command separator, command substitution, or newline can terminate the generated `export` statement and inject additional shell commands. For example, a value conceptually shaped as: ```text /some/path"; injected_command; # ``` produces startup-file content shaped as: ```bash export PATH="$PATH:/some/path"; injected_command; #" ``` The injected command executes whenever the startup file is sourced, including during later interactive shell sessions. This turns a nominal path parameter into a persistent code-execution channel. The same issue affects both Bash and Zsh because the installer selects either `.bashrc` or `.zshrc` as the output file. ### Attack Path 1. An attacker influences the arguments passed to ...[truncated 1497 chars]- Remediation
View remediation
&2 exit 1 } INSTALL_DIR=$2 case "$INSTALL_DIR" in *$'\n'*|*$'\r'*) printf '%s\n' "Error: installation path contains invalid characters" >&2 exit 1 ;; esac shift 2 ;; ``` 2. Serialize the path using Bash-compatible shell escaping rather than embedding it inside manually constructed quotes: ```bash printf -v QUOTED_INSTALL_DIR '%q' "$INSTALL_DIR" printf '\n# preflight workflow package\n' >> "$SHELL_RC" printf 'export PATH="$PATH":%s\n' "$QUOTED_INSTALL_DIR" >> "$SHELL_RC" ``` If Zsh portability is required, avoid relying solely on Bash-specific `%q`. Prefer a fixed launcher directory with a validated character set or create a symbolic link in a standard user-local binary directory such as `$HOME/.local/bin`. 3. Apply a conservative validation policy where practical, for example permitting only expected path characters: ```bash case "$INSTALL_DIR" in *[!A-Za-z0-9_./\ -]*) printf '%s\n' "Error: unsupported characters in installation path" >&2 exit 1 ;; esac ``` 4. Avoid editing startup files by default. Ask for explicit confirmation or provide the exact configuration line for the user to review and add manually. 5. Use a unique marker comment for idempotency instead of the broad `grep -q "preflight"` check. The current test can be triggered by unrelated text and may incorrectly skip installation: ```bash MARKER='# preflight-workflow managed entry' if ! grep -Fqx "$MARKER" "$SHELL_RC" 2>/dev/null; then printf '\n%s\n' "$MARKER" >> "$SHELL_RC" # Append safely serialized configuration. fi ``` 6. Add regression tests using paths containing spaces, ...[truncated 191 chars]