T09 · Insecure Skill Coding Practices
Error
- Location
scripts/script.sh:137- Finding
Arbitrary Command Execution Through Sed Program Injection
- Content
View full analysis
)"; fi elif [ -z "$val" ]; then grep "^${key}=" "$cfg" 2>/dev/null | cut -d= -f2- || echo "(not set)" else if [ -f "$cfg" ] && grep -q "^${key}=" "$cfg" 2>/dev/null; then sed -i "s|^${key}=.*|${key}=${val}|" "$cfg" else echo "${key}=${val}" >> "$cfg" fi echo "Set: $key=$val" fi } ``` ### Technical Analysis The `key` and `val` command-line arguments are interpolated directly into a GNU sed program: ```bash sed -i "s|^${key}=.*|${key}=${val}|" "$cfg" ``` Shell quoting prevents ordinary shell metacharacters inside these values from being interpreted directly by the calling shell. It does not, however, prevent the values from changing the syntax of the sed program itself. An attacker can include the `|` delimiter and newline characters in `val`. This can terminate the intended substitution and introduce an additional GNU sed `e` instruction. GNU sed's `e` instruction executes a shell command, turning sed-language injection into arbitrary operating-system command execution. The vulnerable branch is reached when the configuration file already contains a line matching the supplied key. The `key` is also interpolated into grep and sed regular expressions without escaping, creating additional regular-expression and sed-program manipulation opportunities. ### Attack Path 1. The attacker invokes the Skill's `config` command with a normal key and value, causing that ke ...[truncated 1533 chars]- Remediation
View remediation
&2 return 1 fi ``` 2. Replace the sed-based update with exact string processing that treats the key and value as data. Write the updated configuration to a securely created temporary file and atomically rename it: ```bash tmp=$(mktemp "${cfg}.XXXXXX") trap 'rm -f "$tmp"' EXIT awk -F= -v wanted="$key" -v replacement="$val" ' BEGIN { found = 0 } $1 == wanted { print wanted "=" replacement found = 1 next } { print } END { if (!found) print wanted "=" replacement } ' "$cfg" > "$tmp" chmod 600 "$tmp" mv -- "$tmp" "$cfg" trap - EXIT ``` 3. Reject carriage returns, newlines, NUL bytes, and other control characters in values if the configuration format is intended to remain one record per line. 4. Use `grep -F` or exact field comparisons rather than interpolating keys into regular expressions. 5. Add regression tests containing sed delimiters, backslashes, newlines, regex metacharacters, and attempted GNU sed commands. ]]>