T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/setup.sh:37
- Finding
- Arbitrary Shell Command Execution Through Unsafely Generated Configuration<![CDATA[ ## Vulnerability Details **File Location**: `scripts/setup.sh:37-43`; execution sink documented at `SKILL.md:26-30` **Vulnerability Type**: Shell command injection through unsafe configuration serialization and sourcing **Risk Level**: High ### Vulnerable Code `scripts/setup.sh:37-43`: ```bash cat > "$CONFIG_FILE" <<EOF ACORN_LIB=$ACORN_LIB ACORN_PROJECT=$ACORN_PROJECT USE_MISE=$USE_MISE EOF ``` `SKILL.md:26-30`: ```bash source skills/acorn-prover/config.env ``` ### Technical Analysis The setup script accepts `ACORN_LIB` and `ACORN_PROJECT` as user-supplied arguments. It validates only that each argument identifies an existing directory: ```bash if [[ ! -d "$ACORN_LIB" ]]; then echo "Error: Directory '$ACORN_LIB' does not exist." exit 1 fi ``` Directory names on Unix-like systems may contain spaces, semicolons, command substitutions, and newline characters. The validated values are written verbatim and without shell-safe quoting into `config.env`. The documented and mandatory workflow subsequently executes: ```bash source skills/acorn-prover/config.env ``` Because `source` parses the configuration file as executable shell code, shell metacharacters or additional lines embedded in a crafted directory name are interpreted as commands. The directory-existence check does not prevent this because an attacker can create a real directory whose name contains the malicious syntax. This also causes reliability problems for benign paths containing spaces or other shell-significant characters, since the generated assignments may no longer represent the original paths correctly. ### Attack Path 1. An attacker creates a valid directory with a crafted name containing a newline followed by a shell command. 2. The attacker supplies or persuades the user to supply that directory as the `ACORN_LIB` or `ACORN_PROJECT` setup argument. 3. The `[[ -d "$VALUE" ]]` check succeeds because the crafted directory actually exists. 4. `scripts/setup.sh` writes the ...[truncated 1210 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Avoid storing configuration data in a file that is later interpreted as shell code. Prefer a non-executable format such as JSON and load it using a parser that returns data without evaluating shell syntax. If shell assignments must be retained: 1. Serialize each value with Bash-safe escaping: ```bash { printf 'ACORN_LIB=%q\n' "$ACORN_LIB" printf 'ACORN_PROJECT=%q\n' "$ACORN_PROJECT" printf 'USE_MISE=%q\n' "$USE_MISE" } > "$CONFIG_FILE" ``` 2. Reject control characters, particularly carriage returns and newlines, before writing values: ```bash if [[ "$ACORN_LIB" == *$'\n'* || "$ACORN_LIB" == *$'\r'* || "$ACORN_PROJECT" == *$'\n'* || "$ACORN_PROJECT" == *$'\r'* ]]; then echo "Error: Paths must not contain newline characters." >&2 exit 1 fi ``` 3. Canonicalize and revalidate both paths with an appropriate mechanism such as `realpath`, while preserving strict quoting. 4. Write the configuration atomically with restrictive permissions, such as mode `0600`, to reduce local tampering risks. 5. Do not use `source` for attacker-influenced files. If possible, read expected keys as data and reject unknown or duplicate entries. 6. Add regression tests covering spaces, quotes, semicolons, command substitutions, backslashes, and newline characters in path arguments. ]]>