T09 · Insecure Skill Coding Practices
Error
- Location
scripts/script.sh:137- Finding
Arbitrary Command Execution Through GNU 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 ``` ### Technical Analysis The user-controlled `key` and `val` parameters are inserted directly into a dynamically constructed `sed` program. Double-quoting the shell argument prevents shell word splitting, but it does not make the resulting text safe for the `sed` interpreter. An attacker can place the `|` delimiter and newline characters in a value to terminate the intended substitution and append another GNU `sed` command. GNU `sed` supports the `e` command, which executes a command through a shell. Consequently, crafted configuration input can change the `sed` program from a data update into arbitrary local command execution. The configuration key is also interpreted as a regular expression by both `grep` and `sed`, further demonstrating that untrusted configuration data is being treated as executable syntax rather than literal text. ### Attack Path 1. The attacker creates an ordinary configuration entry so that `config.txt` exists and the target key follows the update branch: ```bash scripts/script.sh config demo initial ``` 2. The attacker updates the same key with a value containing a delimiter, newline, and GNU ` ...[truncated 1038 chars]- Remediation
View remediation
&2 return 1 } ``` - Reject carriage returns, newlines, NUL-equivalent input, and other control characters in configuration values unless explicitly required. - Replace the dynamic `sed` update with a data-oriented implementation. For example, read the file line by line, compare keys as literal strings, and write results to a securely created temporary file. - Create temporary files inside `DATA_DIR` with `mktemp`, restrictive permissions, and an exit trap; then atomically rename the completed file. - If `sed` remains in use, escape both regular-expression and replacement syntax. This is less robust than avoiding dynamic interpreter input and should not be the primary defense. - Add regression tests containing delimiters, backslashes, regular-expression metacharacters, and embedded newlines. ]]>