T09 · Insecure Skill Coding Practices
Error
- Location
scripts/script.sh:136- Finding
Arbitrary Command Execution Through GNU sed Expression 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` substitution program. Neither value is escaped for the regular-expression, replacement-string, delimiter, or command contexts. An attacker can include the `|` delimiter and a newline in an argument to terminate the intended substitution and inject additional `sed` syntax. In GNU `sed`, the `e` substitution flag executes the resulting replacement text through a shell. A crafted replacement can therefore cause an attacker-selected shell command to run. The same code also permits regular-expression manipulation through `key`, but command execution through the replacement and GNU `sed` execution flag is the most severe consequence. ### Attack Path 1. The attacker invokes the `config` command with a benign key and value so that a matching entry is created in `config.txt`. 2. The attacker invokes `config` again for the same key, ensuring execution enters the update branch containing `sed -i`. 3. The attacker supplies a value containing a delimiter, the GNU `sed` `e` flag, and a n ...[truncated 1095 chars]- Remediation
View remediation
&2 return 1 fi ``` - Treat values exclusively as data. Rewrite the configuration through a safely quoted parser and a temporary file rather than using dynamic `sed` expressions. - Create the temporary file inside the data directory with `mktemp`, restrictive permissions, and an exit trap, then atomically replace the original file. - If `sed` must be retained, separately escape regular-expression metacharacters in the key and delimiter, backslash, ampersand, and newline characters in the replacement. Strictly reject control characters. A parser-based rewrite is preferable because escaping dynamic `sed` syntax is error-prone. - Add regression tests with delimiters, ampersands, backslashes, newlines, regular-expression syntax, and strings resembling `sed` flags. ]]>