T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/crawlora.sh:45
- Finding
- Overbroad HTTP Method and Product Route Authorization<![CDATA[ ## Vulnerability Details **File Location**: `scripts/crawlora.sh`, lines 45–65 and 95–100 **Vulnerability Type**: Overbroad API request authorization **Risk Level**: Medium ### Vulnerable Code ```bash case "$method" in GET|POST) ;; *) echo "only GET and POST are supported by the hm-research skill" >&2 exit 2 ;; esac # Reject path syntax that could smuggle a route through a shell glob check. case "$path" in ""|*[?#%]*|*..*|*//* ) echo "invalid path for the hm-research skill" >&2 exit 2 ;; esac case "$path" in /hm/categories) ;; /hm/listing) ;; /hm/product/*) ;; /hm/product/*/related) ;; /hm/search) ;; /hm/search/suggestions) ;; /hm/stores) ;; *) echo "path is not in the hm-research skill catalog" >&2 exit 2 ;; esac ``` ```bash else [ -n "$body" ] || body="${rest[0]:-}" [ -n "$body" ] || body='{}' # Stream the body on stdin so curl never interprets a user value as its # @file shorthand (and cannot read local files supplied in a request body). printf '%s' "$body" | curl -fsS -X "$method" "${auth[@]}" \ -H "Content-Type: application/json" --data-binary @- "${base}${path}" fi ``` ### Technical Analysis The endpoint reference declares seven read-only H&M operations, all using HTTP `GET`. The helper nevertheless accepts both `GET` and `POST` for every allowlisted path. It then forwards an arbitrary JSON body with the user's `CRAWLORA_API_KEY` when `POST` is selected. The product route validation is also broader than the documented API: ```bash /hm/product/*) ;; /hm/product/*/related) ;; ``` The first pattern accepts any nonempty suffix, including nonnumeric product identifiers and additional path segments. It already subsumes the second pattern because shell `*` can match slashes. This does not enforce the documented forms: - `/hm/product/{numeric_product_id}` - `/hm/product/{numeric_product_id}/related` Consequently, the helper can authenticate undocumented POST request ...[truncated 2009 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Reject every method except `GET`, because all documented endpoints are read-only: ```bash [ "$method" = "GET" ] || { echo "only GET is supported by the hm-research skill" >&2 exit 2 } ``` 2. Replace broad shell-glob authorization with endpoint-specific validation. Require product IDs to contain digits only and distinguish the detail and related routes explicitly: ```bash case "$path" in /hm/categories|/hm/listing|/hm/search|/hm/search/suggestions|/hm/stores) ;; /hm/product/[0-9]*) product_suffix="${path#/hm/product/}" case "$product_suffix" in *[!0-9]*|"") echo "invalid product path" >&2; exit 2 ;; esac ;; /hm/product/[0-9]*/related) product_id="${path#/hm/product/}" product_id="${product_id%/related}" case "$product_id" in *[!0-9]*|"") echo "invalid related-product path" >&2; exit 2 ;; esac ;; *) echo "path is not in the hm-research skill catalog" >&2 exit 2 ;; esac ``` 3. Prefer a strict regular-expression check in Bash for the two dynamic routes: ```bash if [[ "$path" =~ ^/hm/product/[0-9]+(/related)?$ ]]; then : fi ``` 4. Bind each endpoint to its documented method rather than validating methods and paths independently. This prevents future changes from accidentally authorizing unsafe method/path combinations. 5. Add negative tests covering POST requests, nonnumeric product IDs, nested product paths, encoded separators, traversal syntax, query fragments, and undocumented routes. ]]>