T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/download_results.py:58
- Finding
- Unrestricted URL Fetching Enables Server-Side Request Forgery<![CDATA[ ## Vulnerability Details **File Location**: `scripts/download_results.py`, lines 58–71, 92, 101, and 130 **Vulnerability Type**: Server-Side Request Forgery and unbounded remote-content download **Risk Level**: High ### Vulnerable Code ```python def download_file(url, filepath): """Download a single file.""" req = urllib.request.Request(url, headers={"User-Agent": "LibTV-Skill/1.0"}) try: with urllib.request.urlopen(req, timeout=60) as resp: with open(filepath, "wb") as f: while True: chunk = resp.read(8192) if not chunk: break f.write(chunk) return filepath, None except Exception as e: return filepath, str(e) ``` The command-line interface accepts URLs directly and submits each URL to this function: ```python parser.add_argument( "--urls", nargs="+", default=[], help="Directly specify a list of URLs to download", ) urls = list(args.urls) # ... futures = { pool.submit(download_file, url, fp): (url, fp) for url, fp in tasks } ``` ### Technical Analysis The `--urls` option accepts arbitrary user-controlled URLs. These URLs are passed directly to `urllib.request.urlopen()` without validating: - The URL scheme - The destination hostname - The resolved IP address - Redirect destinations - Whether the target is a loopback, link-local, private, multicast, or reserved address - The response media type - The response size Although URLs extracted from assistant text are partially constrained by a LibTV-domain regular expression, the documented `--urls` path bypasses that restriction entirely. Python's `urllib` also follows HTTP redirects by default, so validation limited to an initial URL would not be sufficient. The vulnerability is especially relevant in an Agent Skill because an untrusted prompt can induce the Agent to invoke the documented download command with attacker-sel ...[truncated 1515 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Restrict downloads to `https` URLs. 2. Use an explicit hostname allowlist, such as the exact approved LibTV result host. 3. Resolve the hostname before connecting and reject every address that is loopback, link-local, private, multicast, unspecified, or reserved. 4. Disable redirects or validate the scheme, hostname, and resolved address after every redirect. 5. Reject URLs containing embedded credentials or ambiguous hostname encodings. 6. Enforce an allowlist of expected media content types. 7. Set a maximum response size and stop writing when the limit is reached. 8. Apply both connection and read deadlines. 9. Download to a temporary file and atomically move it into place only after all checks succeed. 10. Consider removing arbitrary `--urls` support if downloading non-LibTV resources is not required. Example validation policy: ```python from urllib.parse import urlparse import ipaddress import socket ALLOWED_HOSTS = {"libtv-res.liblib.art"} MAX_DOWNLOAD_BYTES = 500 * 1024 * 1024 def validate_download_url(url): parsed = urlparse(url) if parsed.scheme != "https": raise ValueError("Only HTTPS URLs are permitted") if parsed.hostname not in ALLOWED_HOSTS: raise ValueError("Unapproved download host") for result in socket.getaddrinfo(parsed.hostname, 443): address = ipaddress.ip_address(result[4][0]) if ( address.is_private or address.is_loopback or address.is_link_local or address.is_multicast or address.is_reserved or address.is_unspecified ): raise ValueError("Unsafe destination address") ``` Equivalent checks must be repeated for redirect targets, and DNS rebinding should be mitigated by connecting to the validated address or otherwise binding validation to the actual connection. ]]>