T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/bookmark.mjs:646
- Finding
- Unrestricted Base URL Enables Server-Side Request Forgery<![CDATA[ ## Vulnerability Details **File Location**: `scripts/bookmark.mjs:3`, `scripts/bookmark.mjs:341-375`, `scripts/bookmark.mjs:646-652`, `scripts/bookmark.mjs:685` **Vulnerability Type**: Unrestricted network destination / Server-Side Request Forgery (SSRF) **Risk Level**: Medium ### Vulnerable Code ```js const DEFAULT_BASE_URL = process.env.BOOKMARK_BASE_URL?.trim() || "https://shuqianlan.com"; ``` ```js class ShuqianlanClient { constructor(baseUrl) { this.baseUrl = new URL(baseUrl); } async fetchJson(url) { const response = await fetch(url, { method: "GET", headers: { Accept: "application/json", }, }); if (response.status === 404) { return undefined; } if (!response.ok) { throw new Error(`request failed: ${response.status}`); } return response.json(); } async fetchText(url) { const response = await fetch(url, { method: "GET", headers: { Accept: "text/html,application/xhtml+xml", }, }); if (response.status === 404) { return ""; } if (!response.ok) { throw new Error(`request failed: ${response.status}`); } return response.text(); } ``` ```js if (current === "--base-url") { const next = args.shift(); if (!readString(next)) { throw new Error("`--base-url` requires a URL."); } options.baseUrl = next.trim(); continue; } ``` ```js const client = new ShuqianlanClient(options.baseUrl); ``` ### Technical Analysis The Skill is declared as a read-only client for the public `https://shuqianlan.com` bookmark service. However, the request origin can be replaced through either the `BOOKMARK_BASE_URL` environment variable or the `--base-url` command-line option. The implementation verifies only that the supplied value is a nonempty string and can be parsed by `new URL()`. It does not enforce: - The expected `shuqianlan.com` hostname. - An allowlist of trusted origins. - HTTPS as the required proto ...[truncated 3120 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Remove arbitrary endpoint overrides if they are not operationally required.** Keep the service origin fixed: ```js const DEFAULT_BASE_URL = "https://shuqianlan.com"; ``` 2. **If overrides are required, enforce an exact origin allowlist.** ```js const ALLOWED_ORIGINS = new Set([ "https://shuqianlan.com", ]); function validateBaseUrl(value) { const url = new URL(value); if (url.protocol !== "https:") { throw new Error("Only HTTPS bookmark endpoints are allowed."); } if (url.username || url.password) { throw new Error("URL credentials are not allowed."); } if (!ALLOWED_ORIGINS.has(url.origin)) { throw new Error("The bookmark endpoint is not trusted."); } return url; } ``` 3. **Do not trust an inherited environment variable by default.** Require an explicit, validated configuration mechanism when a nondefault endpoint is genuinely needed. 4. **If arbitrary enterprise deployments must be supported, block unsafe destinations after DNS resolution.** Reject loopback, private, link-local, multicast, reserved, and cloud metadata address ranges for both IPv4 and IPv6. 5. **Validate every redirect hop or disable redirects.** Use `redirect: "manual"` and permit a redirect only after validating its destination with the same policy. 6. **Apply egress controls outside the application.** Restrict the Skill runtime so it can connect only to the approved bookmark service over HTTPS. 7. **Add security tests** covering environment overrides, CLI overrides, URL credentials, non-HTTPS protocols, loopback addresses, private addresses, IPv6 local addresses, DNS rebinding scenarios, and redirects to internal destinations. ]]>