T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/security-audit.sh:61
- Finding
- Command Injection Through Dynamically Constructed SSH Configuration Summary<![CDATA[ ## Vulnerability Details **File Location**: `scripts/security-audit.sh`, lines 61-99 **Vulnerability Type**: OS command injection through `bash -c` **Risk Level**: High ### Vulnerable Code ```bash local permit_root="$(echo "$sshd_config" | grep -iE '^\s*PermitRootLogin\s' | awk '{print $2}')" if [[ "$permit_root" == "yes" || "$permit_root" == "prohibit-password" || "$permit_root" == "without-password" ]]; then warn "PermitRootLogin is set to '$permit_root' (consider 'no')" elif [[ "$permit_root" == "no" ]]; then pass "PermitRootLogin is 'no'" else info "PermitRootLogin not explicitly set (default may allow root)" fi local password_auth="$(echo "$sshd_config" | grep -iE '^\s*PasswordAuthentication\s' | awk '{print $2}')" if [[ "$password_auth" == "yes" || -z "$password_auth" ]]; then warn "PasswordAuthentication is enabled or not set (consider key-only auth)" elif [[ "$password_auth" == "no" ]]; then pass "PasswordAuthentication is disabled" fi local port="$(echo "$sshd_config" | grep -iE '^\s*Port\s' | awk '{print $2}')" if [[ -n "$port" ]]; then info "SSH is listening on port $port" else info "SSH is on default port 22" fi local protocol="$(echo "$sshd_config" | grep -iE '^\s*Protocol\s' | awk '{print $2}')" if [[ -n "$protocol" && "$protocol" != "2" ]]; then fail "Protocol is '$protocol' (should be 2)" fi gather_section "SSH Configuration Summary" bash -c "echo 'PermitRootLogin: ${permit_root:-not set}'; echo 'PasswordAuthentication: ${password_auth:-not set}'; echo 'Port: ${port:-22}'" ``` ### Technical Analysis The script reads values from `/etc/ssh/sshd_config` and `/etc/ssh/sshd_config.d/*.conf`, extracts selected fields, and interpolates them directly into a command string passed to `bash -c`. Although the generated `echo` arguments use single quotes, the configuration values themselves are not shell-escaped. An attacker-controlled value containing a single quote can terminate the intended quoted argument ...[truncated 2306 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Remove the dynamically generated `bash -c` invocation. These values do not require shell evaluation and should be appended to the report as plain data. A safe replacement is: ```bash gather_ssh_summary() { printf 'PermitRootLogin: %s\n' "${permit_root:-not set}" printf 'PasswordAuthentication: %s\n' "${password_auth:-not set}" printf 'Port: %s\n' "${port:-22}" } gather_section "SSH Configuration Summary" gather_ssh_summary ``` Alternatively, pass values as positional arguments to a fixed command rather than interpolating them into shell source: ```bash gather_section "SSH Configuration Summary" \ bash -c 'printf "PermitRootLogin: %s\nPasswordAuthentication: %s\nPort: %s\n" "$1" "$2" "$3"' \ _ "${permit_root:-not set}" "${password_auth:-not set}" "${port:-22}" ``` The first approach is preferred because it eliminates the unnecessary nested shell entirely. Additional hardening measures: - Treat every value read from system configuration as untrusted data. - Avoid `eval`, dynamically generated shell commands, and interpolated `bash -c` strings. - Validate extracted values against strict allowlists. For example, require `Port` to be numeric and within the valid TCP port range. - Parse only effective, syntactically valid SSH configuration where possible, such as by using `sshd -T` under controlled conditions. - Ensure `/etc/ssh/sshd_config` and configuration fragments are owned by root and are not writable by unprivileged users or groups. - Add regression tests containing quotes, semicolons, command substitutions, and redirection characters to confirm that configuration values are always handled as data. ]]>