T09 · Insecure Skill Coding Practices
Error
- Location
scripts/script.sh:137- Finding
Arbitrary Command Execution Through GNU sed Configuration 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 direct shell expansion while constructing the argument, but it does not make untrusted text safe as sed syntax. An attacker can place sed delimiters, regular-expression metacharacters, backslashes, or newline characters in either argument. In particular, a newline in `val` can terminate the intended substitution and insert another sed command. GNU sed supports the `e` command, which executes a supplied command through a shell. Because the script uses GNU-style `sed -i`, an injected `e` command can lead to arbitrary command execution under the identity running the Skill. The same unescaped `key` is also used as a regular expression by `grep` and sed. This permits unintended matching or modification of configuration entries even when command execution is not attempted. ### Attack Path 1. The attacker invokes the exposed `config` command to create a normal existing key: ```bash scripts/script.sh ...[truncated 1408 chars]- Remediation
View remediation
&2 return 1 } ``` - Reject newline, carriage-return, NUL, and other control characters in configuration values. - Replace the sed-based update with a safe implementation that compares keys as data rather than executable syntax. For example, parse the file line by line, write validated entries to a newly created temporary file, and atomically rename it over the original. - Create temporary files inside `DATA_DIR` using `mktemp`, set restrictive permissions, and use `trap` to remove them on failure. - If a text-processing utility is retained, pass values through data variables and ensure they cannot become program syntax. Do not rely only on escaping one delimiter because regular expressions, replacement syntax, newlines, and implementation-specific commands must all be handled. - Add regression tests covering delimiters, backslashes, regular-expression metacharacters, embedded newlines, carriage returns, and GNU sed commands such as `e`. ]]>