T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/script.sh:145
- Finding
- Arbitrary Command Execution Through GNU sed Program Injection## 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**: High ```bash 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 `key` and `val` originate 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, the `e` command 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 in `key` can also alter the regular expression an ...[truncated 1803 chars]
- Remediation
- ## Remediation Suggestions Do not construct a sed program by interpolating untrusted configuration data. 1. 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 } ``` 2. 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. 3. Use `mktemp`, restrictive permissions, and an atomic `mv` after the complete file has been written successfully. 4. Preserve values without evaluating them as shell, regular-expression, or sed syntax. 5. 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. 6. Add regression tests covering newlines, `|`, backslashes, ampersands, regular-expression metacharacters, and attempted `e` command injection.