T09 · Insecure Skill Coding Practices
Error
- Location
- logic.ts:318
- Finding
- Server-Side Request Forgery Through Unrestricted Article URL Fetching<![CDATA[ ## Vulnerability Details **File Location**: `logic.ts:318-327` and `logic.ts:476-482` **Vulnerability Type**: Server-Side Request Forgery (SSRF) **Risk Level**: High ### Vulnerable Code ```ts async function getArticle(url: string): Promise<string> { try { // Fetch from Zoomin API with proper headers to get JSON with full HTML const response = await fetch(url, { headers: { 'Accept': 'application/json', }, redirect: 'follow', }); ``` The tool definition exposes the URL directly to callers: ```ts export const servicenow_get_article: ToolDef = { name: 'servicenow_get_article', description: 'Fetch the full content of a ServiceNow documentation article', schema: z.object({ url: z.string().describe('The article URL from search results'), }), execute: async (args: unknown) => { const { url } = args as { url: string }; return getArticle(url); }, }; ``` ### Technical Analysis The `servicenow_get_article` tool accepts an arbitrary string as its `url` argument and passes it directly to the server-side `fetch` API. The implementation does not: - Parse and validate the supplied URL. - Require the HTTPS protocol. - Restrict requests to approved ServiceNow hostnames. - Reject localhost, private, link-local, loopback, or reserved network addresses. - Restrict destination ports. - Resolve and validate destination IP addresses. - Validate redirect destinations. The request explicitly uses `redirect: 'follow'`. Consequently, even validation of only the initial hostname would remain vulnerable if an approved or attacker-controlled endpoint redirected the request to an internal destination. The `toPublicUrl()` helper does not mitigate this vulnerability because it is only used when formatting returned output. It neither validates nor transforms the URL before the network request occurs. ### Attack Path 1. An attacker or untrusted prompt invokes `servicenow_get_article` with a URL targeting an intern ...[truncated 1424 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Use an exact hostname allowlist** - Parse input with `new URL(url)`. - Require `https:`. - Permit only the exact ServiceNow documentation hosts required by the feature, such as `docs.servicenow.com` and the explicitly approved Zoomin backend. - Do not use suffix checks that could accept names such as `docs.servicenow.com.attacker.example`. 2. **Avoid arbitrary URLs** - Prefer accepting a validated article identifier or relative documentation path. - Construct the final ServiceNow URL internally from a trusted base URL. 3. **Control redirects** - Set `redirect: 'manual'` and reject redirects, or validate every redirect destination using the same protocol, hostname, port, and address rules. - Apply a strict maximum redirect count. 4. **Block internal network destinations** - Resolve the hostname and reject loopback, private, link-local, multicast, unspecified, and reserved IPv4 and IPv6 ranges. - Repeat validation when connecting and after every redirect to mitigate DNS rebinding and time-of-check/time-of-use issues. - Enforce outbound network restrictions at the container, firewall, or proxy layer as defense in depth. 5. **Restrict ports and response handling** - Permit only TCP port 443. - Set request timeouts and response-size limits. - Validate the response content type and expected JSON structure before processing it. 6. **Strengthen schema validation** - Replace `z.string()` with a URL schema plus application-level validation. - Return a generic validation error that does not reveal internal network details. 7. **Add security tests** - Test rejection of `localhost`, loopback addresses, RFC1918 addresses, link-local metadata addresses, IPv6 local addresses, encoded IP representations, embedded credentials, non-HTTPS schemes, nonstandard ports, deceptive subdomains, and redirects to blocked destinations. ]]>