T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/configure_official_cli.sh:62
- Finding
- Shell Injection Through Unsafely Generated Wrapper Script<![CDATA[ ## Vulnerability Details **File Location**: `scripts/configure_official_cli.sh`, lines 8–10 and 62–76 **Vulnerability Type**: Shell command injection through unescaped configuration values **Risk Level**: High ### Vulnerable Code ```bash VAULT_PATH="$(realpath -m "${1:-/root/obsidian-vault}")" OBSIDIAN_USER="${OBSIDIAN_USER:-obsidian}" WRAPPER_PATH="${WRAPPER_PATH:-/usr/local/bin/obs}" ``` ```bash cat > "$WRAPPER_PATH" <<EOF #!/usr/bin/env bash set -euo pipefail cmd=() for arg in "\$@"; do cmd+=("\$(printf '%q' "\$arg")") done exec su - ${OBSIDIAN_USER} -c "cd ${VAULT_PATH} && xvfb-run -a /usr/bin/obsidian --disable-gpu \${cmd[*]}" EOF chmod +x "$WRAPPER_PATH" ``` ### Technical Analysis The script generates an executable shell wrapper while directly interpolating `VAULT_PATH` and `OBSIDIAN_USER` into shell source code. These values are not encoded with `printf '%q'`, passed as positional arguments, or otherwise constrained to a safe character set. Although `realpath -m` normalizes the vault path, it does not remove shell metacharacters such as double quotes, semicolons, command substitutions, or newlines. A crafted path can therefore terminate the generated `cd` command context and insert additional commands into the wrapper. The arguments passed to the generated wrapper are individually escaped, but that protection does not apply to the values embedded while the wrapper is created. The generated wrapper is normally installed at `/usr/local/bin/obs` and may subsequently be executed by the root-run verification workflow. The same vault path and vault name are also interpolated into JSON without JSON escaping: ```bash cat > "$CONFIG_FILE" <<JSON { "cli": true, "vaults": { "${VAULT_NAME}": { "path": "${VAULT_PATH}", "ts": ${TS}, "open": true } } } JSON ``` This can produce malformed or attacker-controlled configuration fields, although the direct shell injection in the wrapper is the more severe issue. ### Att ...[truncated 1274 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Do not generate executable shell code containing interpolated paths or usernames. - Store the vault path in a root-owned configuration file and read it as data at runtime. - Pass dynamic values as positional parameters rather than inserting them into a `su -c` command string. - If source generation cannot be avoided, encode every inserted shell value with `printf '%q'` before writing it. - Validate `OBSIDIAN_USER` against an appropriate strict username pattern and verify that its home directory is obtained from the system account database rather than constructed from the username. - Generate `obsidian.json` with a JSON-aware tool such as Python or `jq` so quotes, backslashes, control characters, and newlines are correctly escaped. - Add regression tests using vault paths containing spaces, quotes, semicolons, dollar signs, command substitutions, and newlines. - Avoid invoking the generated wrapper as root during verification when root privileges are not required. ]]>