T09 · Insecure Skill Coding Practices
Warning
- Location
- examples/find-by-date.sh:29
- Finding
- Unsafe JSON Construction Allows MCP Query Manipulation## Vulnerability Details **File Location**: `examples/find-by-date.sh:29-39`; `examples/find-people-together.sh:52-57`; `examples/get-photo-urls.sh:37-42` **Vulnerability Type**: JSON injection through unvalidated string interpolation **Risk Level**: Medium ### Vulnerable Code `examples/find-by-date.sh:29-39`: ```bash QUERY="{ \"body_createdAfter\": \"$START_ISO\", \"body_createdBefore\": \"$END_ISO\", \"query_order\": \"desc\", \"query_size\": $LIMIT" if [[ -n "$CITY" ]]; then QUERY="$QUERY, \"body_city\": \"$CITY\"" fi ``` `examples/find-people-together.sh:52-57`: ```bash RESULTS=$(mcporter call immich.immich_searchassets \ --args "{ \"body_personIds\": [\"$PERSON1_ID\", \"$PERSON2_ID\"], \"query_order\": \"desc\", \"query_size\": $LIMIT }" \ ``` `examples/get-photo-urls.sh:37-42`: ```bash RESULTS=$(mcporter call immich.immich_searchassets \ --args "{ \"body_personIds\": [\"$PERSON_ID\"], \"query_order\": \"desc\", \"query_size\": $LIMIT }" \ ``` ### Technical Analysis The scripts construct JSON request bodies by directly interpolating command-line arguments and API-derived values. No JSON escaping is applied to string values, and `LIMIT` is inserted as an unquoted JSON token without validation that it is a bounded integer. A crafted `CITY` value containing quotes and additional JSON properties can alter the generated request structure. Similarly, a crafted `LIMIT` can append properties after an initially valid numeric value. This is JSON-level injection rather than shell command injection because the resulting request is still passed as one quoted argument to `mcporter`. The person IDs are obtained from Immich responses rather than directly from the command line, but they are also interpolated without JSON-safe encoding. A compromised or unexpectedly formatted service response could therefore produce malformed or manipulated requ ...[truncated 1042 chars]
- Remediation
- ## Remediation Suggestions - Construct request bodies with `jq` instead of concatenating JSON strings. - Pass strings through `jq --arg` and numeric values through `jq --argjson`. - Validate `LIMIT` before use, for example by requiring a decimal integer within a reasonable range such as 1–100. - Validate date arguments against the expected `YYYY-MM-DD` format before appending timestamps. - Treat identifiers returned by remote services as untrusted and encode them with the same JSON-safe mechanism. Example hardening pattern: ```bash if ! [[ "$LIMIT" =~ ^[0-9]+$ ]] || (( LIMIT < 1 || LIMIT > 100 )); then echo "Limit must be an integer from 1 to 100." >&2 exit 1 fi QUERY=$(jq -n \ --arg after "$START_ISO" \ --arg before "$END_ISO" \ --arg city "$CITY" \ --argjson size "$LIMIT" \ '{ body_createdAfter: $after, body_createdBefore: $before, query_order: "desc", query_size: $size } + if $city != "" then {body_city: $city} else {} end') ``` Apply the same approach to person-ID arrays: ```bash QUERY=$(jq -n \ --arg first "$PERSON1_ID" \ --arg second "$PERSON2_ID" \ --argjson size "$LIMIT" \ '{ body_personIds: [$first, $second], query_order: "desc", query_size: $size }') ```