T09 · Insecure Skill Coding Practices
- Location
weather-cn.sh:40- Finding
Remote Command Injection Through Evaluation of Untrusted Weather Data
- Content
View full analysis
Vulnerability Details
File Location:
weather-cn.sh, lines 40–48, 63–72, 123–128, and 133–139
Vulnerability Type: Shell command injection througheval
Risk Level: HighVulnerable Code
bash fetch_weather() { local city_code="$1" local url="https://www.weather.com.cn/weather/${city_code}.shtml" # Fetch HTML content local html html=$(curl -s --max-time 10 "$url" 2>/dev/null) if [ -z "$html" ]; then error "Unable to retrieve weather data" fi echo "$html" }bash # Extract weather information from the remote HTML title local weather weather=$(cat "$tmpfile" | grep -o '<title>[^<]*</title>' | sed 's/<title>//' | sed 's/天气预报.*//' | head -1) # If unavailable in the title, extract it from the HTML body if [ -z "$weather" ] || [ "$weather" = "未知" ]; then weather=$(cat "$tmpfile" | grep -oE '(晴转多云|多云转晴|晴|多云|阴|小雨|大雨|雪|雷阵雨)' | head -1) fibash echo "WEATHER=${weather:-未知}" echo "TEMP=${temp:-未知}" echo "COLD_INDEX=${cold_index}" echo "SPORT_INDEX=${sport_index}" echo "DRESS_INDEX=${dress_index}" echo "WASH_INDEX=${wash_index}" echo "UV_INDEX=${uv_index}"bash format_output() { local city="$1" shift local data="$@" # Parse data eval "$data"Technical Analysis
The script downloads HTML from an external website and extracts the weather description from the remotely controlled
<title>element. The extracted value is serialized into a shell assignment such asWEATHER=<remote content>. The resulting multiline string is then passed directly toeval.evalparses its input as shell source code rather than treating it as inert data. The remote weather value is neither shell-escaped nor constrained to an allowlist before evaluation. Consequently, shell metacharacters, command substitutions, assignment-breaking syntax, or additional commands embedded in the res ...[truncated 2339 chars]- Remediation
View remediation
Remediation Suggestions
-
Remove
evalcompletely. Do not serialize parsed values as shell source code. Pass each value as a quoted function argument, store values in global variables, or use another data-transfer mechanism that does not invoke a shell parser.A safer design is:
bash format_output "$city" "$weather" "$temp" \ "$cold_index" "$sport_index" "$dress_index" \ "$wash_index" "$uv_index"The receiving function should assign each positional parameter with quoting:
bash format_output() { local city="$1" local weather="$2" local temp="$3" local cold_index="$4" local sport_index="$5" local dress_index="$6" local wash_index="$7" local uv_index="$8" # Format values without eval. } -
Strictly validate remote values. Restrict weather descriptions and temperatures to the precise formats needed by the application. Prefer selecting known weather terms rather than accepting arbitrary title text. Reject unexpected control characters, newlines, quotes, backticks, dollar signs, semicolons, parentheses, and other shell syntax as defense in depth.
-
Treat all network responses as untrusted. Keep remote data quoted at every use and never interpret it as executable shell syntax, even when obtained over HTTPS.
-
Harden the HTTP request. Use options such as:
bash curl --fail --silent --show-error --max-time 10 \ --max-filesize 1048576 "$url"Also validate the response type and impose a reasonable response-size limit.
-
Avoid unnecessary temporary-file handling. Parse data directly from standard input where practical. If a temporary file remains necessary, create it with
mktemp, ensure cleanup with a trap, and maintain restrictive permissions:bash tmpfile=$(mktemp) || exit 1 trap 'rm -f "$tmpfile"' RETURN
...[truncated 250 chars]
-