T09 · Insecure Skill Coding Practices
- Location
- scripts/health-check-recovery.sh:21
- Finding
- Recurring Arbitrary Command Execution Through Sourced Configuration<![CDATA[ ## Vulnerability Details **File Location**: `scripts/health-check-recovery.sh:21-30` **Related Location**: `scripts/install.sh:69-87` **Vulnerability Type**: Shell command injection through unsafe configuration loading **Risk Level**: High ### Vulnerable Code ```bash CONFIG_FILE="$HOME/.openclaw/config/keepalive.conf" # Telegram notification configuration (read from configuration file) if [ -f "$CONFIG_FILE" ]; then source "$CONFIG_FILE" fi ``` The installer places unvalidated interactive input into the sourced file: ```bash read -r BOT_TOKEN echo "Please enter Telegram Chat ID (format: 123456789):" read -r CHAT_ID cat > ~/.openclaw/config/keepalive.conf <<EOF # OpenClaw Keepalive configuration file # Automatically generated by the installer # Telegram notification configuration TELEGRAM_BOT_TOKEN="$BOT_TOKEN" TELEGRAM_CHAT_ID="$CHAT_ID" EOF ``` ### Technical Analysis The `source` command evaluates the complete contents of `keepalive.conf` as shell code. The file is intended to contain two data values, but no parser, key allowlist, syntax validation, or character validation separates configuration data from executable shell syntax. The installer also writes user-provided values directly into shell assignment statements. A value containing a closing quotation mark followed by a shell command can generate a syntactically valid malicious configuration. For example, an input shaped like: ```text "; touch "$HOME/.openclaw/injected"; # ``` can result in executable content when the recovery script later sources the file. The health-check script is registered as a LaunchAgent and runs every 60 seconds. Consequently, modification of this configuration file provides a recurring execution path rather than a one-time injection. This does not cross into root privileges because the LaunchAgent is installed in the current user's GUI domain. Nevertheless, it permits arbitrary command execution with all permissions of the logged-in user. ### Att ...[truncated 1272 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove `source "$CONFIG_FILE"` entirely. 2. Store configuration in a non-executable format such as JSON or plist. 3. Parse only an explicit allowlist of keys: - `TELEGRAM_BOT_TOKEN` - `TELEGRAM_CHAT_ID` 4. Reject duplicate keys, unknown fields, newlines, control characters, shell metacharacters, and malformed values. 5. Validate the Chat ID as an expected numeric identifier and validate the token against a conservative format before storage. 6. Pass parsed values directly to `curl`; never reconstruct shell syntax or use `eval`. 7. Verify that the configuration is a regular file owned by the current user and is not a symbolic link before reading it. 8. If shell-format configuration must be retained, use a strict inert parser that extracts values without evaluating the file. Merely filtering some shell characters before using `source` is not a robust fix. 9. Add regression tests using quotation marks, semicolons, command substitutions, backticks, newlines, and malformed assignments to confirm that none are executed. ]]>
