T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/public_api.py:12
- Finding
- Unrestricted API Base URL Enables Server-Side Request Forgery<![CDATA[ ## Vulnerability Details **File Location**: `scripts/public_api.py`, lines 12–14, 29–36, and 80–83 **Vulnerability Type**: Server-Side Request Forgery through an unrestricted network destination **Risk Level**: Medium ### Vulnerable Code ```python def build_env_config(env: str, base_url: Optional[str]) -> EnvConfig: """Resolve final base URL from env / override.""" if base_url: return EnvConfig(base_url=base_url.rstrip("/")) ``` ```python def http_get_json(url: str) -> Dict[str, Any]: """Perform a GET request and return parsed JSON.""" req = request.Request(url, method="GET") req.add_header("Content-Type", "application/json") try: with request.urlopen(req, timeout=15) as resp: status = resp.getcode() body = resp.read().decode("utf-8") ``` ```python parser.add_argument( "--base-url", help="Optional: custom API root URL; takes precedence when set.", ) ``` ### Technical Analysis The `--base-url` argument accepts an arbitrary URL and gives it precedence over the two documented Stove API environments. No validation restricts the URL scheme, hostname, port, resolved IP address, or redirect destination. The resulting value is combined with an API path and passed to `urllib.request.urlopen`. Python's standard URL opener follows HTTP redirects by default. Consequently, a caller can direct the skill to an attacker-controlled endpoint, a loopback service, a private-network host, or a link-local service. An attacker-controlled server can also redirect the request to a destination that could not conveniently be addressed using the appended Stove API path. The implementation does not enforce HTTPS and does not reject loopback, private, link-local, multicast, or reserved IP ranges. It also does not revalidate redirect targets. These behaviors violate least-destination principles for a skill whose documented network activity only requires access to two fixed Stove API hosts. The ticker ...[truncated 2382 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove `--base-url` if custom destinations are not operationally necessary. 2. Prefer a strict allowlist containing only: - `https://proto.stove.finance` - `https://api-qa.proto.stove.finance` 3. Parse candidate URLs with `urllib.parse.urlsplit` and require: - The `https` scheme. - An allowlisted hostname. - No embedded username or password. - No unexpected port. - No fragment component. 4. Disable automatic redirects, or validate every redirect destination against the same scheme, hostname, port, and address restrictions before following it. 5. If arbitrary hosts must be supported, resolve all destination addresses and reject loopback, private, link-local, multicast, unspecified, and reserved IPv4 and IPv6 ranges. Protect against DNS rebinding by ensuring the validated address is the address used for the connection. 6. Validate ticker symbols against the supported format, for example an explicit length-limited allowlist of uppercase letters, digits, dots, or hyphens. 7. Percent-encode the ticker path segment independently: ```python safe_symbol = parse.quote(args.symbol, safe="") path = f"/api/v1/tickers/{safe_symbol}/stats" ``` 8. Add automated tests covering direct private addresses, IPv6 loopback, encoded addresses, user-information syntax, non-HTTPS schemes, unusual ports, malicious ticker path segments, and redirects to prohibited destinations. ]]>