T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/search_jobs.sh:31
- Finding
- Arbitrary Code Execution Through Python Source Injection<![CDATA[ ## Vulnerability Details **File Location**: `scripts/search_jobs.sh`, lines 31, 67, 91-101, and 104-109 **Vulnerability Type**: Python source-code injection caused by unsafe shell interpolation **Risk Level**: High ### Vulnerable Code The user-controlled search query is embedded directly inside Python source: ```bash ENCODED_QUERY=$(python3 -c "import urllib.parse; print(urllib.parse.quote('$QUERY'))") ``` The derived search query is embedded through the same unsafe pattern: ```bash for sq in "${SEARCH_QUERIES[@]}"; do ENCODED_SQ=$(python3 -c "import urllib.parse; print(urllib.parse.quote('$sq'))") ``` JSON assembled from Brave Search API responses is also inserted into executable Python source: ```bash ALL_RESULTS=$(python3 -c " import json, sys a = json.loads('$ALL_RESULTS') b = json.loads(sys.stdin.read()) seen = set(x['url'] for x in a) for item in b: if item['url'] not in seen: a.append(item) seen.add(item['url']) print(json.dumps(a)) " <<< "$PARSED" 2>/dev/null || echo "$ALL_RESULTS") ``` Finally, the user-controlled `--limit` value is inserted as an executable Python expression: ```bash # Trim to limit echo "$ALL_RESULTS" | python3 -c " import json, sys results = json.load(sys.stdin)[:${LIMIT}] print(json.dumps(results, indent=2)) " ``` ### Technical Analysis The script builds Python programs using shell-expanded strings and passes them to `python3 -c`. Values such as `QUERY`, `sq`, `ALL_RESULTS`, and `LIMIT` are treated as Python source code instead of data. Quoting the shell variable does not make interpolation into Python source safe. An attacker can include Python quote delimiters, statement separators, expressions, and comments in an argument. Once interpolated, these characters can terminate the intended string or slice expression and append arbitrary Python statements. The vulnerable data flows are: 1. The first positional command-line argument is assigned to `QUERY`. 2. `QUERY` is placed inside a s ...[truncated 4084 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Never construct executable Python source by interpolating shell or remote values. Pass all values as command-line arguments, environment variables, or standard input. ### 1. Pass search strings through `sys.argv` Replace line 31 with: ```bash ENCODED_QUERY=$( python3 -c \ 'import sys, urllib.parse; print(urllib.parse.quote(sys.argv[1]))' \ "$QUERY" ) ``` Replace line 67 with: ```bash ENCODED_SQ=$( python3 -c \ 'import sys, urllib.parse; print(urllib.parse.quote(sys.argv[1]))' \ "$sq" ) ``` In both cases, the Python program remains constant and the untrusted value is handled only as data. ### 2. Validate numeric arguments strictly Validate `DAYS` and `LIMIT` after argument parsing: ```bash if [[ ! "$DAYS" =~ ^[0-9]+$ ]]; then printf 'Error: --days must be a non-negative integer\n' >&2 exit 2 fi if [[ ! "$LIMIT" =~ ^[0-9]+$ ]]; then printf 'Error: --limit must be a non-negative integer\n' >&2 exit 2 fi ``` Pass the validated limit as an argument rather than interpolating it: ```bash echo "$ALL_RESULTS" | python3 -c ' import json import sys limit = int(sys.argv[1]) results = json.load(sys.stdin)[:limit] print(json.dumps(results, indent=2)) ' "$LIMIT" ``` A reasonable upper bound should also be enforced to prevent excessive resource consumption: ```bash if (( LIMIT > 100 )); then printf 'Error: --limit must not exceed 100\n' >&2 exit 2 fi ``` ### 3. Merge JSON without embedding it into source Provide both JSON documents through files, separate file descriptors, or arguments. A robust option is to use temporary files created securely: ```bash tmp_all=$(mktemp) tmp_parsed=$(mktemp) trap 'rm -f "$tmp_all" "$tmp_parsed"' EXIT printf '%s' "$ALL_RESULTS" > "$tmp_all" printf '%s' "$PARSED" > "$tmp_parsed" ALL_RESULTS=$( python3 - "$tmp_all" "$tmp_parsed" <<'PY' import json import sys with open(sys.argv[1], encoding="utf-8") as handle: existing = js ...[truncated 1134 chars]