T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/zhipu_fetch.sh:21
- Finding
- Unsafe JSON Construction Allows Request-Body Injection<![CDATA[ ## Vulnerability Details **File Location**: `scripts/zhipu_fetch.sh`, lines 21–22 and 59–77 **Vulnerability Type**: JSON request-body injection through unvalidated command-line arguments **Risk Level**: Medium ### Vulnerable Code ```bash -t|--timeout) TIMEOUT="$2"; shift ;; -f|--format) RETURN_FORMAT="$2"; shift ;; ``` ```bash # Escape special characters in URL for safe JSON encoding SAFE_URL="${URL//\\/\\\\}" SAFE_URL="${SAFE_URL//\"/\\\"}" # Build JSON payload PAYLOAD=$(cat <<EOF { "url": "$SAFE_URL", "timeout": $TIMEOUT, "no_cache": $NO_CACHE, "return_format": "$RETURN_FORMAT", "retain_images": $RETAIN_IMAGES, "no_gfm": $NO_GFM, "keep_img_data_url": $KEEP_IMG_DATA_URL, "with_images_summary": $WITH_IMAGES_SUMMARY, "with_links_summary": $WITH_LINKS_SUMMARY } EOF ) ``` ### Technical Analysis The script constructs JSON by directly interpolating command-line arguments into a here-document. `TIMEOUT` is inserted as an unquoted raw JSON value, while `RETURN_FORMAT` is placed inside a JSON string without escaping or allowlist validation. An attacker who can control these arguments can terminate the intended value and inject additional JSON members. For example, a crafted format value could introduce another `timeout` property: ```bash bash scripts/zhipu_fetch.sh \ --url "https://www.example.com" \ --format 'markdown", "timeout": 999999, "injected":"' ``` This can produce a request containing attacker-introduced fields or duplicate properties. The behavior of duplicate properties depends on the API's JSON parser, but many parsers accept the last occurrence, potentially allowing an injected value to override an earlier legitimate value. The URL receives only partial escaping. Backslashes and double quotes are escaped, but JSON control characters such as literal newlines, carriage returns, and tabs are not encoded. Such input can make the request malformed. This is not shell command injection because `PAYLOAD` is passed to `cu ...[truncated 1592 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate every command-line option before constructing the request: - Require `TIMEOUT` to contain digits only. - Enforce a reasonable timeout range, such as 1–300 seconds. - Allow only `markdown` or `text` for `RETURN_FORMAT`. - Reject missing values for options that require an argument. 2. Build the request with a real JSON encoder rather than string interpolation. For example: ```bash [[ "$TIMEOUT" =~ ^[0-9]+$ ]] || { echo "Error: timeout must be an integer" >&2 exit 1 } if (( TIMEOUT < 1 || TIMEOUT > 300 )); then echo "Error: timeout must be between 1 and 300 seconds" >&2 exit 1 fi case "$RETURN_FORMAT" in markdown|text) ;; *) echo "Error: format must be markdown or text" >&2 exit 1 ;; esac PAYLOAD=$(jq -n \ --arg url "$URL" \ --argjson timeout "$TIMEOUT" \ --argjson no_cache "$NO_CACHE" \ --arg return_format "$RETURN_FORMAT" \ --argjson retain_images "$RETAIN_IMAGES" \ --argjson no_gfm "$NO_GFM" \ --argjson keep_img_data_url "$KEEP_IMG_DATA_URL" \ --argjson with_images_summary "$WITH_IMAGES_SUMMARY" \ --argjson with_links_summary "$WITH_LINKS_SUMMARY" \ '{ url: $url, timeout: $timeout, no_cache: $no_cache, return_format: $return_format, retain_images: $retain_images, no_gfm: $no_gfm, keep_img_data_url: $keep_img_data_url, with_images_summary: $with_images_summary, with_links_summary: $with_links_summary }') ``` 3. If adding `jq` is undesirable, use another available JSON serializer that correctly escapes quotes, backslashes, Unicode, and control characters. Avoid manually assembling JSON. 4. Add tests covering quotes, backslashes, newlines, tabs, duplicate-property injection attempts, nonnumeric timeouts, extreme timeout values, missing argument values, and unsupported output formats. ]]>