T09 · Insecure Skill Coding Practices
Error
- Location
analyze.sh:20- Finding
User-Controlled Python Code Injection in Search Query Encoding
- Content
View full analysis
/dev/null | \ python3 -c " import sys, re, html content = sys.stdin.read() results = re.findall(r'class=\"result__snippet\">(.*?)', content, re.DOTALL) for i, r in enumerate(results[:5]): text = html.unescape(re.sub(r'<[^>]+>', '', r)).strip() if text: print(f'- {text}') " 2>/dev/null || echo "- (search unavailable — run with an AI agent for better results)" } ``` ### Technical Analysis The `query` variable contains data derived from the user-supplied company name or URL. It is interpolated directly into Python source code inside a single-quoted Python string: ```bash python3 -c "import urllib.parse; print(urllib.parse.quote('$query'))" ``` Shell double quotes do not make the expanded value safe for use as Python source code. If `query` contains an apostrophe, it can terminate the Python string literal. An attacker can then append additional Python statements and use a Python comment to suppress the remaining generated source. The sanitization applied while deriving `SEARCH_NAME` only removes URL protocols, path components, and `www.`. It does not escape apostrophes or prevent Python syntax from entering the dynamically constructed program. This is a code-injection vulnerability rather than ordinary shell metacharacter injection: the shell passes one constructed argument to `python3 -c`, and the Python interpreter executes attacker-controlled content embedded in that argument. ### Attack Path 1. An attacker supplies a crafted company name containing an apostrophe, valid Python statements, and a trailing Python comment marker. 2. ...[truncated 1524 chars]- Remediation
View remediation
(.*?)", content, re.DOTALL) for result in results[:5]: text = html.unescape(re.sub(r"<[^>]+>", "", result)).strip() if text: print(f"- {text}") ' } ``` Prefer letting `curl` perform URL encoding directly with `--get` and `--data-urlencode`; in that case, `encoded_query` is unnecessary and should be removed. Additional hardening measures should include: 1. Treat all command-line arguments as untrusted data. 2. Avoid constructing executable source code with shell variable expansion. 3. Add regression tests using company names containing apostrophes, quotes, semicolons, comment characters, Unicode, and shell metacharacters. 4. Use `curl --fail --show-error` so HTTP and transport failures are distinguishable from successful searches. 5. Run the Skill with least privilege and without unnecessary credentials in its environment. 6. Consider validating input length to prevent excessive resource use, while recognizing that validation is not a substitute for safe argument passing. ]]>