T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/bootstrap_restic_home.sh:98
- Finding
- Root Command Injection Through Shell-Sourced Configuration<![CDATA[ ## Vulnerability Details **File Location**: `scripts/bootstrap_restic_home.sh:27-31, 98-103, 109, 116, 123, 215-216` **Vulnerability Type**: Shell command injection through an unsafe generated configuration file **Risk Level**: High ### Vulnerable Code ```bash --repo) REPO="$2"; shift 2 ;; --password-file) PASS_FILE="$2"; shift 2 ;; --timezone) TIMEZONE="$2"; shift 2 ;; ``` ```bash cat >/etc/restic-home.env <<EOF RESTIC_REPOSITORY=${REPO} RESTIC_PASSWORD_FILE=${PASS_FILE} BACKUP_SOURCE=${HOME_DIR} EXCLUDES_FILE=/etc/restic-home/excludes.txt EOF chmod 600 /etc/restic-home.env ``` The generated scripts later interpret this file as shell code: ```bash #!/usr/bin/env bash set -euo pipefail source /etc/restic-home.env exec /usr/bin/restic backup "$BACKUP_SOURCE" --exclude-file "$EXCLUDES_FILE" ``` The repository initialization path also sources it directly: ```bash if [[ "$INIT_REPO" == "yes" ]]; then source /etc/restic-home.env if ! /usr/bin/restic snapshots >/dev/null 2>&1; then /usr/bin/restic init fi fi ``` ### Technical Analysis The values accepted through `--repo`, `--password-file`, and indirectly `--user` are inserted into `/etc/restic-home.env` without escaping or validation. Although systemd supports an `EnvironmentFile` format, the generated backup, prune, and check scripts do not parse the file strictly as systemd environment data. They execute: ```bash source /etc/restic-home.env ``` Consequently, the file is interpreted as shell code. An attacker-controlled argument containing a newline can append a new shell statement to the generated file. Shell metacharacters placed in a separate injected line will execute when the file is sourced. The bootstrap script is intended to be run with elevated privileges because it writes to `/etc`, `/usr/local/bin`, and `/etc/systemd/system`. The generated systemd services do not specify a less-privileged `User=`, so they run as root by default. This converts configuration injection into ...[truncated 1673 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not execute configuration files with `source`. Treat configuration as data rather than shell code. 2. Store values in a parser-safe format and load them with a parser that does not evaluate shell expressions. 3. Alternatively, pass fixed, validated values directly to the generated scripts using safely quoted arguments. 4. Reject carriage returns, newlines, NUL-equivalent input, and other control characters in every command-line value. 5. Validate `--user` against the system account database rather than constructing `/home/${USER_NAME}` directly. 6. Validate the repository according to an explicit allowlist of supported restic repository formats and transport schemes. 7. Require `--password-file` to be an absolute path under an approved directory, such as `/etc/restic-home/`. 8. If a systemd `EnvironmentFile` remains necessary, generate it using correct systemd escaping and ensure no shell script sources it. 9. Add explicit least-privilege service settings where supported. At minimum, consider: - `User=` and `Group=` appropriate for the backup source - `NoNewPrivileges=true` - `PrivateTmp=true` - `ProtectSystem=strict` - Narrow `ReadWritePaths=` entries for locations that genuinely require writes 10. Add tests using values containing newlines, quotes, semicolons, command substitutions, and shell redirection to verify that no supplied value can become executable syntax. ]]>