T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/setup.sh:100
- Finding
- Arbitrary Command Execution and rclone Option Injection Through Unsafe Configuration Handling<![CDATA[ ## Vulnerability Details **File Location**: `scripts/setup.sh:64`, `scripts/setup.sh:100-104`, `scripts/sync.sh:39-45`, and `scripts/sync.sh:67-68` **Vulnerability Type**: Shell command injection and command-line option injection **Risk Level**: High ### Vulnerable Code `scripts/setup.sh:64` accepts unrestricted extra command-line flags: ```bash prompt EXTRA_FLAGS "Extra rclone flags" "--transfers=4 --checkers=8" ``` `scripts/setup.sh:100-104` writes user-controlled values directly into an executable shell configuration file without escaping or validation: ```bash cat > "$ENV_FILE" <<EOF RCLONE_REMOTE=${RCLONE_REMOTE} BACKUP_KEEP=${BACKUP_KEEP} RCLONE_EXTRA_FLAGS="${EXTRA_FLAGS}" EOF chmod 600 "$ENV_FILE" ``` `scripts/sync.sh:39-45` executes the generated configuration as shell code: ```bash load_env() { [[ -f "$ENV_FILE" ]] || die "config missing — run: $0 setup" # shellcheck disable=SC1090 set -a; source "$ENV_FILE"; set +a : "${RCLONE_REMOTE:?RCLONE_REMOTE not set in $ENV_FILE}" : "${BACKUP_KEEP:=1}" : "${RCLONE_EXTRA_FLAGS:=}" } ``` `scripts/sync.sh:67-68` additionally expands the configured flags as an unquoted scalar: ```bash [[ -f "$FILTER_FILE" ]] && filter_args=(--filter-from "$FILTER_FILE") rclone --config "$RCLONE_CONFIG_FILE" "${filter_args[@]}" $RCLONE_EXTRA_FLAGS "$@" ``` ### Technical Analysis The setup wizard treats interactive values as data, but serializes them directly into `config/.env`, which is subsequently loaded with Bash `source`. A sourced file is executable shell code rather than a passive configuration format. Values such as `RCLONE_REMOTE`, `BACKUP_KEEP`, and `EXTRA_FLAGS` are not restricted or shell-escaped before being written. Newlines, quotation marks, command substitutions, or other shell syntax can therefore alter the structure of the generated file and introduce commands that execute when `load_env` sources it. For example, a malicious value containing a newline can terminate an assignme ...[truncated 2473 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Do not source configuration files** - Replace the executable `.env` format with a passive format such as JSON, TOML, or a strictly parsed key-value file. - Parse only recognized keys and never evaluate configuration content as shell code. 2. **Apply strict input validation** - Restrict `RCLONE_REMOTE` to a conservative pattern such as `^[A-Za-z0-9_-]+$`. - Require `BACKUP_KEEP` to contain only an integer within an appropriate range. - Reject newline, carriage-return, NUL, and other control characters in every configuration value. - Validate endpoint, bucket, prefix, and crypt-mode fields according to their expected formats. 3. **Eliminate unrestricted extra flags** - Prefer explicit configuration prompts for supported settings such as transfer and checker counts. - If extra flags remain necessary, enforce an allowlist of safe rclone options and reject options capable of changing configuration, remote access, filtering, logging, command execution, or filesystem scope. 4. **Use array-safe argument construction** - Store approved options as separate array elements. - Invoke rclone using quoted array expansion: ```bash local extra_flags=( "--transfers=$TRANSFERS" "--checkers=$CHECKERS" ) rclone \ --config "$RCLONE_CONFIG_FILE" \ "${filter_args[@]}" \ "${extra_flags[@]}" \ "$@" ``` 5. **Protect fixed security arguments** - Reject user-provided options such as `--config`, `--filter`, `--filter-from`, `--include`, `--exclude`, `--log-file`, or other flags that can override security-sensitive arguments. - Construct mandatory arguments after validating all optional settings, while avoiding reliance solely on argument ordering. 6. **Write configuration atomically** - Create configuration files in a securely permissioned temporary file inside `config/`. - Validate the completed file, apply mode `600`, and atomically rename it into place. - Continue using `umask 077` to pr ...[truncated 335 chars]