T09 · Insecure Skill Coding Practices
Error
- Location
scripts/script.sh:130- Finding
Arbitrary Command Execution Through sed Script 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 embedded directly into a `sed` program: ```bash sed -i "s|^${key}=.*|${key}=${val}|" "$cfg" ``` Neither value is escaped for use in a regular expression, replacement expression, or sed script. Shell quoting prevents a direct shell metacharacter injection at the initial expansion stage, but it does not prevent the expanded argument from introducing sed syntax. In particular, Bash command-line arguments can contain newline characters. On sed implementations that support command execution, such as GNU sed, an attacker can terminate the substitution command with the `|` delimiter and inject a new `e` command on the following line. GNU sed's `e` command executes a shell command. The unescaped `key` is also used as a regular expression in `grep` and `sed`, allowing unintended matching or configuration manipulation even when command execution is not available. ### Attack Path 1. The attacker must be able to invoke the skill or influence arguments passed to its `config` command. 2. The attacker first creates an ordinary c ...[truncated 1352 chars]- Remediation
View remediation
&2 return 1 } ``` - Reject carriage returns, newlines, NUL-equivalent input, and other control characters in configuration values. - Replace the dynamic sed expression with a parser that passes data separately from program text. For example, use `awk` variables with validation and write the result to a securely created temporary file. - Prefer a structured format such as JSON and update it through a serializer such as `jq`, rather than treating configuration data as executable regular-expression syntax. - Create temporary files inside the configured data directory with `mktemp`, set restrictive permissions, and atomically rename the completed file. - Add regression tests containing sed delimiters, backslashes, newlines, regular-expression metacharacters, and attempted `e` commands. ]]>