T09 · Insecure Skill Coding Practices
Error
- Location
- hooks/scripts/check-config.sh:27
- Finding
- Automatic Command Injection Through Project-Controlled Configuration<![CDATA[ ## Vulnerability Details **File Location**: `hooks/scripts/check-config.sh:27-39`; automatically invoked by `hooks/hooks.json:2-12` **Vulnerability Type**: Shell command injection through unsafe `eval` **Risk Level**: High ### Vulnerable Code ```bash load_env_vars() { local file="$1" if [[ -f "$file" ]]; then while IFS='=' read -r key value; do [[ "$key" =~ ^[[:space:]]*# ]] && continue [[ -z "$key" ]] && continue key=$(echo "$key" | xargs) value=$(echo "$value" | xargs | sed 's/^["'\''"]//;s/["'\''"]$//') if [[ -n "$key" && -n "$value" ]]; then eval "ENV_${key}=\"${value}\"" fi done < "$file" fi } ``` The vulnerable script is registered as an unrestricted session-start hook: ```json { "hooks": { "SessionStart": [ { "matcher": "", "hooks": [ { "type": "command", "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/check-config.sh", "timeout": 5 } ] } ] } } ``` ### Technical Analysis The hook reads `.claude/last30days-cn.env` from the current project and interpolates its keys and values into a shell expression executed by `eval`. Neither the key nor the value is validated or safely escaped. Quoting the value while constructing the `eval` argument does not make it safe. When `eval` reparses the resulting command, shell constructs embedded in the configuration value—including command substitutions—are interpreted as executable shell syntax. This is particularly dangerous because the project-level file takes precedence over the global configuration and may be supplied by an untrusted repository. The hook runs automatically at session startup with an empty matcher, so exploitation does not require the user to invoke the research command. ### Attack Path 1. An attacker creates or modifies `.claude/last30days-cn.env` in a repository. 2. The attacker places shell syntax, such as command subs ...[truncated 1173 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove `eval` entirely. 2. Permit only explicitly supported configuration names through an allowlist. 3. Validate keys against a strict identifier expression such as `^[A-Z][A-Z0-9_]*$`. 4. Assign values as data rather than executable shell text. For example: ```bash case "$key" in SETUP_COMPLETE|WEIBO_ACCESS_TOKEN|SCRAPECREATORS_API_KEY|ZHIHU_COOKIE|\ TIKHUB_API_KEY|WECHAT_API_KEY|BAIDU_API_KEY) printf -v "ENV_${key}" '%s' "$value" ;; *) printf 'Ignoring unsupported configuration key: %s\n' "$key" >&2 ;; esac ``` 5. Do not use `xargs` as a general-purpose configuration parser because it alters quoting and whitespace. 6. Consider removing project-level configuration processing from the automatic `SessionStart` hook. Parse it only when the user explicitly invokes the Skill. 7. If automatic project configuration remains supported, require user confirmation before trusting a repository-provided file. 8. Add regression tests containing command substitutions, backticks, semicolons, quotes, newlines, and malformed variable names, and verify that none are executed. ]]>