T09 · Insecure Skill Coding Practices
Error
- Location
- install.sh:8
- Finding
- Unsanitized installation paths enable persistent shell command injection<![CDATA[ ## Vulnerability Details **File Location**: `install.sh`, lines 8–16 and 37–42 **Vulnerability Type**: Shell command injection through unsafe source-code rewriting **Risk Level**: High ### Vulnerable Code ```bash # Get configuration from user read -p "Obsidian vault path [/root/ObsidianVault/Clawd Markdowns]: " VAULT_PATH VAULT_PATH=${VAULT_PATH:-/root/ObsidianVault/Clawd Markdowns} read -p "Session directory [/root/.clawdbot/agents/main/sessions]: " SESSION_DIR SESSION_DIR=${SESSION_DIR:-/root/.clawdbot/agents/main/sessions} read -p "Tracking directory [/root/clawd]: " TRACKING_DIR TRACKING_DIR=${TRACKING_DIR:-/root/clawd} ``` ```bash # Update paths in scripts for script in scripts/*.sh; do sed -i "s|VAULT_DIR=\".*\"|VAULT_DIR=\"$VAULT_PATH\"|g" "$script" sed -i "s|/root/.clawdbot/agents/main/sessions|$SESSION_DIR|g" "$script" sed -i "s|/root/clawd|$TRACKING_DIR|g" "$script" done ``` ### Technical Analysis The installer accepts three arbitrary path strings and directly inserts them into executable shell scripts using `sed`. It does not validate the input or escape shell syntax, newlines, backslashes, the `sed` delimiter, or replacement-string metacharacters. Although shell syntax introduced through variable expansion is not recursively evaluated by the currently running installer, it is written into the target scripts. It is then interpreted as shell code the next time one of those modified scripts runs. For example, a vault path containing a command substitution expression can cause the installer to generate an assignment structurally equivalent to: ```bash VAULT_DIR="$(attacker-controlled-command)" ``` The command substitution executes when the generated script starts. The same issue affects session and tracking directory input, and the replacement loop modifies every shell script in `scripts/`. This vulnerability is especially significant because the project instructs the user to run `monitor_and_save.sh` hourly through cro ...[truncated 1848 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Do not rewrite executable source files with configuration values.** Store paths in a dedicated configuration file and have each script load the configuration from a fixed, trusted location. 2. **Avoid evaluating configuration as shell code.** Prefer a data-only format such as JSON and retrieve values with `jq`. If a shell configuration file is retained, create it using safe serialization such as: ```bash { printf 'VAULT_DIR=%q\n' "$VAULT_PATH" printf 'SESSION_DIR=%q\n' "$SESSION_DIR" printf 'TRACKING_DIR=%q\n' "$TRACKING_DIR" } > config ``` 3. **Validate path input.** Reject control characters and newlines, require absolute paths where appropriate, and enforce an explicit policy for allowed characters and locations. 4. **If `sed` remains necessary, escape values separately for the replacement context.** At minimum, escape backslashes, ampersands, and the selected delimiter. This is less robust than separating code from configuration and does not replace shell-safe serialization. 5. **Validate generated scripts before use.** Run `bash -n` on every generated or modified shell script and abort installation if validation fails. 6. **Apply least privilege.** Recommend installation and cron execution under a dedicated unprivileged account with read-only access to the required session directory and write access only to the selected vault and tracking directories. Replace root-oriented defaults with user-scoped paths. 7. **Harden scheduled execution.** Use absolute paths, a restricted `PATH`, restrictive file permissions, and verify that the script and configuration are owned by the scheduling user and are not writable by other accounts. ]]>