T09 · Insecure Skill Coding Practices
- Location
scripts/script.sh:138- Finding
OS Command Injection Through Dynamically Constructed GNU sed Program
- Content
View full analysis
Vulnerability Details
File Location:
scripts/script.sh:138-151
Vulnerability Type: OS command injection
Risk Level: HighVulnerable Code
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
The configuration key and value are attacker-controlled command-line arguments. When an existing key is updated, both arguments are interpolated directly into a GNU sed program:
bash sed -i "s|^${key}=.*|${key}=${val}|" "$cfg"Shell quoting prevents immediate shell expansion of the arguments, but it does not make them safe as sed syntax. A value can contain the
|delimiter and literal newline characters, terminate the intended substitution, and introduce additional sed commands. GNU sed supports theecommand, which executes an external command through a shell.The key is also interpreted as a regular expression by both
grepandsed. Consequently, regex metacharacters can alter which configuration records are selected or replaced, even when command execution is not achieved.Attack Path
- The attacker causes a normal configuration entry to exist, for example by invoking:
bash scripts/script.sh config mode safe - The attacker supplies a replacement value containing a sed de ...[truncated 1039 chars]
- The attacker causes a normal configuration entry to exist, for example by invoking:
- Remediation
View remediation
Remediation Suggestions
- Do not construct sed programs by interpolating untrusted strings.
- Restrict configuration keys to an explicit syntax, such as:
bash [[ "$key" =~ ^[A-Za-z0-9_.-]+$ ]] || { echo "Invalid configuration key" >&2 return 1 } - Rewrite the configuration through a securely created temporary file using a parser that passes values as data rather than executable syntax.
- If
awkis used, pass values with-vand compare keys as strings; do not concatenate them into source code. - Atomically replace the original configuration only after successful generation and validation.
- Reject embedded carriage returns, newlines, and NUL-like data from keys and values unless multiline values are explicitly supported through a safe serialization format.
- Store configuration in a structured format and use a dedicated serializer or parser instead of regular-expression substitution.