T09 · Insecure Skill Coding Practices
- Location
scripts/script.sh:145- Finding
Arbitrary Command Execution Through GNU sed Program Injection
- Content
View full analysis
Vulnerability Details
File Location:
scripts/script.sh, lines 138-150, with the vulnerable command at lines 145-146
Vulnerability Type: Shell command execution through unsafe construction of a GNU sed program
Risk Level: Highbash cmd_config() { local key="${1:-}" val="${2:-}" local cfg="$DATA_DIR/config.txt" if [ -z "$key" ]; then echo "=== Config ===" if [ -f "$cfg" ]; then while IFS="=" read -r k v; do echo " $k=$v"; done < "$cfg" else echo " (empty — use config <key> <value>)"; 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
Both
keyandvaloriginate from command-line arguments and are interpolated directly into a double-quoted sed program. No validation or escaping is applied for sed delimiters, regular-expression metacharacters, replacement metacharacters, backslashes, or newline characters.An attacker can place a newline in
val, terminate the intended substitution with the|delimiter, and append another sed command. On GNU sed, theecommand executes a shell command. For example, a value shaped like the following can turn the generated sed program into multiple commands:text benign| e touch /tmp/cmms-proof #The resulting program is conceptually equivalent to:
sed s|^existing=.*|existing=benign| e touch /tmp/cmms-proof #|The vulnerability is reached only through the update branch, so the selected configuration key must already match an entry in
config.txt. Input inkeycan also alter the regular expression an ...[truncated 1803 chars]- Remediation
View remediation
Remediation Suggestions
Do not construct a sed program by interpolating untrusted configuration data.
- Validate configuration keys against a strict allowlist, such as:
bash [[ "$key" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]] || { echo "Invalid configuration key" >&2 return 1 } - Replace the sed update with a parser that treats keys and values exclusively as data. Write the revised configuration into a securely created temporary file in the same directory.
- Use
mktemp, restrictive permissions, and an atomicmvafter the complete file has been written successfully. - Preserve values without evaluating them as shell, regular-expression, or sed syntax.
- If sed must be retained, escape every character significant to both the search expression and replacement expression, reject carriage returns and newlines, and avoid GNU sed extensions capable of command execution. A structured implementation without dynamically generated sed code is preferred.
- Add regression tests covering newlines,
|, backslashes, ampersands, regular-expression metacharacters, and attemptedecommand injection.
- Validate configuration keys against a strict allowlist, such as: