T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/proxy_fetch.sh:24
- Finding
- Unsafe JSON Construction from Unvalidated Command-Line Arguments## Vulnerability Details **File Location**: `scripts/proxy_fetch.sh:24-26, 63-84` **Vulnerability Type**: JSON injection and insufficient input validation **Risk Level**: Medium ### Vulnerable Code ```bash while [[ "$#" -gt 0 ]]; do case $1 in -u|--url) URL="$2"; shift ;; -t|--timeout) TIMEOUT="$2"; shift ;; -f|--format) RETURN_FORMAT="$2"; shift ;; --no-cache) NO_CACHE="true" ;; --no-images) RETAIN_IMAGES="false" ;; --no-gfm) NO_GFM="true" ;; --keep-img-data-url) KEEP_IMG_DATA_URL="true" ;; --images-summary) WITH_IMAGES_SUMMARY="true" ;; --links-summary) WITH_LINKS_SUMMARY="true" ;; ``` ```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 creates a JSON request through direct shell-string interpolation rather than using a JSON serializer. The `TIMEOUT` argument is inserted as an unquoted JSON value, and `RETURN_FORMAT` is inserted into a quoted JSON string without JSON escaping. Consequently, a caller that controls these command-line arguments can introduce JSON delimiters, quotes, or additional properties into the payload. The URL escaping is also incomplete for general JSON encoding. It handles backslashes and quotation marks but does not correctly encode control characters such as newlines and carriage returns. Such input can produce invalid JSON. Options that require values access `$2` without first verifying that a value exists. This can ...[truncated 2006 chars]
- Remediation
- ## Remediation Suggestions 1. Construct the payload with a real JSON encoder such as `jq` instead of a heredoc: ```bash PAYLOAD=$(jq -n \ --arg url "$URL" \ --argjson timeout "$TIMEOUT" \ --arg return_format "$RETURN_FORMAT" \ --argjson no_cache "$NO_CACHE" \ --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 }') ``` 2. Validate timeout as a bounded positive integer before JSON construction: ```bash if ! [[ "$TIMEOUT" =~ ^[0-9]+$ ]] || (( TIMEOUT < 1 || TIMEOUT > 300 )); then echo "Error: timeout must be an integer between 1 and 300" >&2 exit 1 fi ``` 3. Restrict the output format to its documented allowlist: ```bash case "$RETURN_FORMAT" in markdown|text) ;; *) echo "Error: format must be markdown or text" >&2 exit 1 ;; esac ``` 4. Before reading `$2`, verify that each value-taking option has a following argument and that it is not another option. 5. Validate the final payload with the chosen JSON encoder before sending it. Consider making curl fail explicitly on HTTP errors by using `--fail-with-body`, while retaining appropriate error reporting.