T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/install-wrapper.sh:57
- Finding
- Shell Code Injection in Generated SSH Wrapper<![CDATA[ ## Vulnerability Details **File Location**: `scripts/install-wrapper.sh:57-76` **Vulnerability Type**: Shell code injection through unsafe script generation **Risk Level**: High ### Vulnerable Code ```bash safe_name=$(printf '%s' "$NAME" | tr -cd 'a-zA-Z0-9._-') [[ -n "$safe_name" ]] || { echo "Invalid wrapper name" >&2; exit 1; } [[ "$REMOTE_BIN" = /* ]] || { echo "remote-bin must be an absolute path" >&2; exit 1; } mkdir -p "$TARGET_DIR" wrapper="$TARGET_DIR/$safe_name" ssh_opts=() if [[ -n "$SSH_KEY" ]]; then ssh_opts+=("-i" "$SSH_KEY" "-o" "IdentitiesOnly=yes") fi if [[ -n "$KNOWN_HOSTS" ]]; then ssh_opts+=("-o" "UserKnownHostsFile=$KNOWN_HOSTS") fi cat > "$wrapper" <<EOF #!/usr/bin/env bash set -euo pipefail remote_cmd=\$(printf '%q ' "$REMOTE_BIN" "\$@") exec ssh ${ssh_opts[*]:-} -T "$HOST" "bash -lc \$remote_cmd" EOF ``` ### Technical Analysis The installer generates an executable Bash script using an unquoted heredoc. Values including `REMOTE_BIN`, `HOST`, `SSH_KEY`, and `KNOWN_HOSTS` are inserted directly into the generated shell source without shell-safe serialization. The only validation applied to `REMOTE_BIN` requires it to begin with `/`. It does not reject quotation marks, command substitutions, newlines, backticks, or other shell syntax. `HOST`, `SSH_KEY`, and `KNOWN_HOSTS` receive no equivalent syntax validation. Consequently, a crafted value can break out of its intended syntactic position in the generated wrapper. The malicious syntax is persisted in the wrapper and can execute locally when that wrapper is subsequently invoked. The array used while running the installer does not protect the generated script because `${ssh_opts[*]}` flattens the options into source text. Quoting information from the original array is therefore lost. ### Attack Path 1. An attacker influences an installation argument such as `--remote-bin`, `--host`, `--ssh-key`, or `--known-hosts`. 2. The value satisfies the limited validation. For exa ...[truncated 998 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Do not interpolate configuration values into executable shell source. - Generate the wrapper from a single-quoted static template and load configuration from a separately created, permission-restricted file. - If values must be embedded, serialize every value with `printf '%q'` before writing it to the wrapper. - Preserve SSH options as a Bash array inside the generated wrapper rather than expanding an installer-side array with `${ssh_opts[*]}`. - Validate `HOST` against an explicit `USER@HOST` or approved SSH alias format. - Require path arguments to be absolute, reject control characters and newlines, and resolve or normalize them where appropriate. - Reject unexpected shell metacharacters in fields that do not legitimately require them. - Write wrappers atomically using a temporary file created in the destination directory, set secure permissions, and then rename the file into place. - Consider installing wrappers with mode `0750` or stricter when they expose sensitive remote capabilities. ]]>