T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:99
- Finding
- Shell Command Injection Through Unsafely Generated guardian.conf<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 99-133; execution sink in `config-lib.sh`, lines 20-21 **Vulnerability Type**: Shell command injection through untrusted configuration values **Risk Level**: High ### Vulnerable Code ```bash Ask the user: "What name should I use for myself in team notifications? (e.g. Claw, MyBot, OpenClaw — press Enter to skip and use the default 'OpenClaw')" Record as `BOT_NAME`. If the user skips, use `OpenClaw`. ``` ```bash SKILL_DIR="$HOME/.openclaw/workspace/skills/gateway-guardian" cat > "$SKILL_DIR/guardian.conf" << GUARDIANCONF # Auto-generated by gateway-guardian installer. Do not upload to GitHub. # Fallback notification target (used when dynamic session detection fails) FALLBACK_CHANNEL={FALLBACK_CHANNEL} FALLBACK_TARGET={FALLBACK_TARGET} # Notification language: zh (Chinese) | en (English) LOCALE={LOCALE} # Bot display name used in staff/team notifications BOT_NAME={BOT_NAME} # Team group/channel notification (optional) # Leave empty to disable. Supported formats: # Feishu: oc_xxx # Telegram: -100xxxxxxxxxx (supergroup/channel numeric id) # Discord: 123456789012345678 (channel id, digits only) # Only effective if the channel is configured and running in OpenClaw. STAFF_GROUP_CHAT_ID= GUARDIANCONF ``` The generated file is subsequently loaded as executable shell code: ```bash _GUARDIAN_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" [ -f "$_GUARDIAN_LIB_DIR/guardian.conf" ] && source "$_GUARDIAN_LIB_DIR/guardian.conf" ``` ### Technical Analysis The installer instructs the Agent to insert user-controlled `BOT_NAME` and conversation-derived notification values directly into an unquoted heredoc. An unquoted heredoc performs command substitution, backtick substitution, and parameter expansion while it is being processed. For example, if a value is inserted as: ```bash BOT_NAME=$(touch /tmp/gateway-guardian-injection) ``` the command substitution can execute immediate ...[truncated 1744 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not store untrusted values in a file that is loaded with `source`. Use JSON, TOML, or another non-executable format and parse it with a strict parser. 2. Generate configuration with a quoted heredoc so that the shell does not expand its body: ```bash cat > "$SKILL_DIR/guardian.conf" <<'GUARDIANCONF' ... GUARDIANCONF ``` 3. Pass values separately rather than replacing placeholders inside executable shell text. 4. If shell configuration is unavoidable, serialize each value with `printf '%q'` before writing it. 5. Enforce strict allowlists: - `FALLBACK_CHANNEL`: one of explicitly supported channel names. - `LOCALE`: exactly `zh` or `en`. - Notification IDs: channel-specific anchored regular expressions. - `BOT_NAME`: a conservative length and character allowlist. 6. Reject newlines, control characters, backticks, dollar signs, semicolons, shell redirection characters, and command-substitution syntax. 7. Create the file with restrictive permissions, such as mode `0600`. 8. Add tests using values containing `$(...)`, backticks, quotes, newlines, semicolons, backslashes, and redirection operators. ]]>