T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/fetch_news.py:24
- Finding
- Server-Side Request Forgery Through Unvalidated Deep-Fetch URLs<![CDATA[ ## Vulnerability Details **File Location**: `scripts/fetch_news.py:24-35, 48-50` **Vulnerability Type**: Server-Side Request Forgery caused by insufficient URL validation **Risk Level**: High ### Vulnerable Code ```python def fetch_url_content(url): """ Fetches the content of a URL and extracts text from paragraphs. Truncates to 3000 characters. """ if not url or not url.startswith('http'): return "" try: response = requests.get(url, headers=HEADERS, timeout=5) response.raise_for_status() soup = BeautifulSoup(response.content, 'html.parser') ``` ```python def enrich_items_with_content(items, max_workers=10): with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: future_to_item = {executor.submit(fetch_url_content, item['url']): item for item in items} ``` The deep-fetch feature is enabled through the documented `--deep` option in `SKILL.md:24, 38, 44, 52`. ### Technical Analysis Article URLs are obtained from remote feeds and user-generated sources, including Hacker News and V2EX. The only validation performed before issuing an HTTP request is: ```python url.startswith('http') ``` This check does not: - Parse and restrict the URL scheme to exact `http` or `https` values. - Reject loopback, private, link-local, reserved, multicast, or unspecified IP addresses. - Protect against hostnames that resolve to internal addresses. - Restrict requests to approved external domains. - Validate redirect destinations. `requests.get()` follows HTTP redirects by default. Consequently, even a URL initially hosted on a public domain can redirect the request to an internal network destination. Concurrent processing also permits several attacker-controlled URLs to be requested during one deep-fetch operation. ### Attack Path 1. An attacker submits or controls an item on a supported user-generated news source. 2. The item contains a direct URL to an internal HTTP servi ...[truncated 1258 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Parse URLs using `urllib.parse.urlsplit()` and permit only exact `http` and `https` schemes. 2. Require a nonempty hostname and reject embedded credentials or malformed authority components. 3. Resolve the hostname before connecting and reject all loopback, private, link-local, multicast, reserved, and unspecified IPv4 and IPv6 addresses using Python's `ipaddress` module. 4. Prevent DNS rebinding by ensuring the validated address is the address actually used for the connection, preferably through a hardened HTTP client or controlled egress proxy. 5. Disable redirects with `allow_redirects=False`, or manually follow redirects while repeating the complete validation process for every destination. 6. Prefer an explicit allowlist of approved content domains when operationally feasible. 7. Run deep fetching in a sandbox with network access restricted to the public Internet and with cloud metadata endpoints blocked. 8. Add response-size and content-type limits to reduce resource consumption and prevent retrieval of unexpected binary data. 9. Add automated tests covering loopback addresses, RFC1918 ranges, link-local addresses, IPv6 private addresses, encoded addresses, DNS aliases, and redirect chains. ]]>