T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/dt_helper.sh:84
- Finding
- Command Injection Through Unescaped Configuration Values<![CDATA[ ## Vulnerability Details **File Location**: `scripts/dt_helper.sh`, lines 84–94; attacker-controlled input enters through lines 343–354 **Vulnerability Type**: Shell command injection through dynamically constructed `sed` expressions **Risk Level**: High ### Vulnerable Code ```bash cfg_set() { local key="$1" local value="$2" mkdir -p "$(dirname "$CONFIG")" touch "$CONFIG" if grep -q "^${key}=" "$CONFIG" 2>/dev/null; then sed -i "s|^${key}=.*|${key}=${value}|" "$CONFIG" else echo "${key}=${value}" >> "$CONFIG" fi } ``` The untrusted values are supplied by the following command handler: ```bash cmd_set() { local kv="$1" if [ -z "$kv" ] || [[ "$kv" != *"="* ]]; then echo "❌ 格式错误，用法: --set KEY=VALUE" >&2 exit 1 fi local key="${kv%%=*}" local value="${kv#*=}" cfg_set "$key" "$value" echo "✅ 已设置 ${key}" } ``` ### Technical Analysis Both `key` and `value` originate from the command-line argument passed to `--set`. They are embedded directly into a GNU `sed` program without escaping regular-expression metacharacters, replacement metacharacters, delimiters, newlines, or command flags. When a configuration key already exists, a value containing the `|` delimiter can terminate the replacement expression and introduce the GNU `sed` `e` flag. The `e` flag executes the substituted pattern space as a shell command. For example, a value shaped like: ```text $(attacker-command)|e ``` can cause the generated expression to resemble: ```bash sed -i 's|^EXISTING_KEY=.*|EXISTING_KEY=$(attacker-command)|e' "$CONFIG" ``` GNU `sed` then evaluates the replacement result through a shell, causing the injected command substitution to execute. Unvalidated keys also permit regular-expression manipulation and configuration-file corruption. ### Attack Path 1. An attacker identifies or predicts a configuration key that already exists. 2. The attacker convinces the user or an Agent workflow to invoke `dt_helper.sh --set` with a cr ...[truncated 863 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not construct a `sed` program from untrusted input. 2. Restrict keys to an explicit allowlist of supported DingTalk configuration names. At minimum, enforce a pattern such as `^[A-Z][A-Z0-9_]*$`. 3. Update the configuration through a parser that treats keys and values exclusively as data. A safely implemented `awk` rewrite or a structured configuration format is preferable. 4. Write changes to a securely created temporary file and atomically rename it over the original file. 5. Reject values containing NUL bytes or line breaks unless multiline values are explicitly supported. 6. If `sed` remains in use, escape regular-expression syntax in keys and delimiter, backslash, and ampersand characters in replacement values. Do not permit user input to reach command flags. 7. Add regression tests using delimiters, backslashes, ampersands, newlines, command substitutions, and GNU `sed` flags to verify that no input is interpreted as executable syntax. ]]>