From f3ee7e271a5d2b345a76767b35d5b2d95e96e1d1 Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Sat, 14 Feb 2026 01:01:25 -0800 Subject: [PATCH] security: Fix command injection vulnerability in env var exports (#1086) CRITICAL: Add validation to prevent command injection via malicious environment variable names in `export "${var_name}=..."` patterns. Vulnerability Details: - All instances of `export "${var_name}=${value}"` where var_name is derived from external sources (manifest.json auth fields, user input, API responses) were vulnerable to command injection - If var_name contained shell metacharacters like `;`, `$()`, or backticks, arbitrary code could be executed - Example exploit: var_name=`FOO; rm -rf /` would execute the rm command Affected Files: - shared/key-request.sh: _try_load_env_var() - var_name from manifest.json - shared/common.sh: _load_token_from_config(), ensure_api_token_with_provider(), _multi_creds_load_config(), _multi_creds_prompt(), _poll_instance_once() - var_name from function parameters - test/record.sh: _load_multi_config_from_file(), _try_load_cloud_config(), _prompt_cloud_creds_interactive() - var_name from test fixtures Fix Applied: - Added regex validation before all export statements: `^[A-Z_][A-Z0-9_]*$` - This allowlist enforces standard POSIX environment variable naming (uppercase letters, digits, underscores only, must start with letter or underscore) - Returns error if validation fails, preventing injection Impact: - While current usage passes hardcoded env var names (e.g., "HCLOUD_TOKEN"), the vulnerability existed in the implementation - manifest.json is currently trusted, but defense-in-depth prevents supply chain attacks or accidental malformed entries - Test infrastructure was also vulnerable to malicious fixture data Agent: security-auditor Co-authored-by: Spawn Refactor Service Co-authored-by: Claude Sonnet 4.5 --- shared/common.sh | 27 +++++++++++++++++++++++++++ shared/key-request.sh | 7 +++++++ test/record.sh | 19 ++++++++++++++++++- 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/shared/common.sh b/shared/common.sh index 8e79ede9..bcad3b86 100644 --- a/shared/common.sh +++ b/shared/common.sh @@ -1794,6 +1794,11 @@ _poll_instance_once() { local ip ip=$(_extract_json_field "${response}" "${ip_py}") if [[ -n "${ip}" ]]; then + # SECURITY: Validate ip_var to prevent command injection + if [[ ! "${ip_var}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then + log_error "SECURITY: Invalid env var name rejected: ${ip_var}" + return 1 + fi export "${ip_var}=${ip}" log_info "${description} ready (IP: ${ip})" return 0 @@ -1870,6 +1875,12 @@ _load_token_from_config() { local env_var_name="${2}" local provider_name="${3}" + # SECURITY: Validate env_var_name to prevent command injection + if [[ ! "${env_var_name}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then + log_error "SECURITY: Invalid env var name rejected: ${env_var_name}" + return 1 + fi + if [[ ! -f "${config_file}" ]]; then return 1 fi @@ -1959,6 +1970,12 @@ ensure_api_token_with_provider() { local token token=$(validated_read "Enter your ${provider_name} API token: " validate_api_token) || return 1 + # SECURITY: Validate env_var_name to prevent command injection + if [[ ! "${env_var_name}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then + log_error "SECURITY: Invalid env var name rejected: ${env_var_name}" + return 1 + fi + export "${env_var_name}=${token}" # Validate with provider API @@ -2057,6 +2074,11 @@ _multi_creds_load_config() { if [[ -z "${value}" ]]; then return 1 fi + # SECURITY: Validate env var name before export + if [[ ! "${env_vars[$i]}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then + log_error "SECURITY: Invalid env var name rejected: ${env_vars[$i]}" + return 1 + fi export "${env_vars[$i]}=${value}" i=$((i + 1)) done <<< "${creds}" @@ -2084,6 +2106,11 @@ _multi_creds_prompt() { local idx for idx in $(seq 0 $((${#env_vars[@]} - 1))); do + # SECURITY: Validate env var name before export + if [[ ! "${env_vars[$idx]}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then + log_error "SECURITY: Invalid env var name rejected: ${env_vars[$idx]}" + return 1 + fi local val val=$(safe_read "Enter ${provider_name} ${labels[$idx]}: ") || return 1 if [[ -z "${val}" ]]; then diff --git a/shared/key-request.sh b/shared/key-request.sh index 0ff3f8af..f29cdf87 100644 --- a/shared/key-request.sh +++ b/shared/key-request.sh @@ -60,6 +60,13 @@ _try_load_env_var() { local var_name="${1}" local config_file="${2}" + # SECURITY: Validate var_name to prevent command injection via export + # Only allow uppercase letters, numbers, and underscores (standard env var naming) + if [[ ! "${var_name}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then + log "SECURITY: Invalid env var name rejected: ${var_name}" + return 1 + fi + # Already set in environment? local current_val="${!var_name:-}" if [[ -n "${current_val}" ]]; then diff --git a/test/record.sh b/test/record.sh index d2feded7..a3d55a28 100644 --- a/test/record.sh +++ b/test/record.sh @@ -233,7 +233,14 @@ except: pass read -ra fields <<< "$vals" local i for i in "${!env_vars[@]}"; do - [[ -n "${fields[$i]:-}" ]] && export "${env_vars[$i]}=${fields[$i]}" + # SECURITY: Validate env var name before export + if [[ -n "${fields[$i]:-}" ]]; then + if [[ ! "${env_vars[$i]}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then + echo "SECURITY: Invalid env var name rejected: ${env_vars[$i]}" >&2 + return 1 + fi + export "${env_vars[$i]}=${fields[$i]}" + fi done return 0 } @@ -317,6 +324,11 @@ try_load_config() { # Standard single-token config if [[ -f "$config_file" ]]; then + # SECURITY: Validate env var name before export + if [[ ! "${env_var}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then + echo "SECURITY: Invalid env var name rejected: ${env_var}" >&2 + return 1 + fi local token token=$(python3 -c "import json,sys; d=json.load(open(sys.argv[1])); print(d.get('api_key','') or d.get('token',''))" "$config_file" 2>/dev/null) || true if [[ -n "${token:-}" ]]; then @@ -394,6 +406,11 @@ prompt_credentials() { fi for var_name in $vars_needed; do + # SECURITY: Validate env var name before using in eval or export + if [[ ! "${var_name}" =~ ^[A-Z_][A-Z0-9_]*$ ]]; then + echo "SECURITY: Invalid env var name rejected: ${var_name}" >&2 + return 1 + fi eval "local current=\"\${${var_name}:-}\"" if [[ -n "$current" ]]; then continue